Skip to content

Commit

Permalink
process: refactor global.queueMicrotask()
Browse files Browse the repository at this point in the history
- Lazy load `async_hooks` in the implementation
- Rename `process/next_tick.js` to `process/task_queues.js`
  and move the implementation of `global.queueMicrotask()`
  there since these methods are conceptually related to
  each other.
- Move the bindings used by `global.queueMicrotask()` into
  `node_task_queue.cc` instead of the generic `node_util.cc`
- Use `defineOperation` to define `global.queueMicrotask()`

PR-URL: #26523
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and danbev committed Mar 11, 2019
1 parent cc2c07c commit 8d669bb
Showing 11 changed files with 92 additions and 96 deletions.
19 changes: 6 additions & 13 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
@@ -123,7 +123,7 @@ process.emitWarning = emitWarning;
const {
nextTick,
runNextTicks
} = NativeModule.require('internal/process/next_tick').setup();
} = NativeModule.require('internal/process/task_queues').setupTaskQueue();

process.nextTick = nextTick;
// Used to emulate a tick manually in the JS land.
@@ -201,7 +201,11 @@ if (!config.noBrowserGlobals) {
defineOperation(global, 'clearTimeout', timers.clearTimeout);
defineOperation(global, 'setInterval', timers.setInterval);
defineOperation(global, 'setTimeout', timers.setTimeout);
setupQueueMicrotask();

const {
queueMicrotask
} = NativeModule.require('internal/process/task_queues');
defineOperation(global, 'queueMicrotask', queueMicrotask);

// Non-standard extensions:
defineOperation(global, 'clearImmediate', timers.clearImmediate);
@@ -395,17 +399,6 @@ function createGlobalConsole(consoleFromVM) {
return consoleFromNode;
}

function setupQueueMicrotask() {
const { queueMicrotask } =
NativeModule.require('internal/queue_microtask');
Object.defineProperty(global, 'queueMicrotask', {
value: queueMicrotask,
writable: true,
enumerable: true,
configurable: true,
});
}

function setupDOMException() {
// Registers the constructor with C++.
const DOMException = NativeModule.require('internal/domexception');
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
@@ -870,7 +870,7 @@ Module.runMain = function() {
return loader.import(pathToFileURL(process.argv[1]).pathname);
})
.catch((e) => {
internalBinding('util').triggerFatalException(e);
internalBinding('task_queue').triggerFatalException(e);
});
} else {
Module._load(process.argv[1], null, true);
Original file line number Diff line number Diff line change
@@ -6,7 +6,9 @@ const {
tickInfo,
// Used to run V8's micro task queue.
runMicrotasks,
setTickCallback
setTickCallback,
enqueueMicrotask,
triggerFatalException
} = internalBinding('task_queue');

const {
@@ -27,7 +29,10 @@ const {
emitDestroy,
symbols: { async_id_symbol, trigger_async_id_symbol }
} = require('internal/async_hooks');
const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
const {
ERR_INVALID_CALLBACK,
ERR_INVALID_ARG_TYPE
} = require('internal/errors').codes;
const FixedQueue = require('internal/fixed_queue');

// *Must* match Environment::TickInfo::Fields in src/env.h.
@@ -130,15 +135,52 @@ function nextTick(callback) {
queue.push(new TickObject(callback, args, getDefaultTriggerAsyncId()));
}

// TODO(joyeecheung): make this a factory class so that node.js can
// control the side effects caused by the initializers.
exports.setup = function() {
// Sets the per-isolate promise rejection callback
listenForRejections();
// Sets the callback to be run in every tick.
setTickCallback(processTicksAndRejections);
return {
nextTick,
runNextTicks
};
let AsyncResource;
function createMicrotaskResource() {
// Lazy load the async_hooks module
if (!AsyncResource) {
AsyncResource = require('async_hooks').AsyncResource;
}
return new AsyncResource('Microtask', {
triggerAsyncId: getDefaultTriggerAsyncId(),
requireManualDestroy: true,
});
}

function queueMicrotask(callback) {
if (typeof callback !== 'function') {
throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
}

const asyncResource = createMicrotaskResource();

enqueueMicrotask(() => {
asyncResource.runInAsyncScope(() => {
try {
callback();
} catch (error) {
// TODO(devsnek) remove this if
// https://bugs.chromium.org/p/v8/issues/detail?id=8326
// is resolved such that V8 triggers the fatal exception
// handler for microtasks
triggerFatalException(error);
} finally {
asyncResource.emitDestroy();
}
});
});
}

module.exports = {
setupTaskQueue() {
// Sets the per-isolate promise rejection callback
listenForRejections();
// Sets the callback to be run in every tick.
setTickCallback(processTicksAndRejections);
return {
nextTick,
runNextTicks
};
},
queueMicrotask
};
38 changes: 0 additions & 38 deletions lib/internal/queue_microtask.js

This file was deleted.

3 changes: 1 addition & 2 deletions node.gyp
Original file line number Diff line number Diff line change
@@ -160,16 +160,15 @@
'lib/internal/process/esm_loader.js',
'lib/internal/process/execution.js',
'lib/internal/process/main_thread_only.js',
'lib/internal/process/next_tick.js',
'lib/internal/process/per_thread.js',
'lib/internal/process/policy.js',
'lib/internal/process/promises.js',
'lib/internal/process/stdio.js',
'lib/internal/process/warning.js',
'lib/internal/process/worker_thread_only.js',
'lib/internal/process/report.js',
'lib/internal/process/task_queues.js',
'lib/internal/querystring.js',
'lib/internal/queue_microtask.js',
'lib/internal/readline.js',
'lib/internal/repl.js',
'lib/internal/repl/await.js',
12 changes: 0 additions & 12 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
@@ -13,7 +13,6 @@ using errors::TryCatchScope;
using v8::Context;
using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Int32;
using v8::Isolate;
@@ -752,15 +751,4 @@ void FatalException(Isolate* isolate,
}
}

void FatalException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
if (env != nullptr && env->abort_on_uncaught_exception()) {
Abort();
}
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
FatalException(isolate, exception, message);
}

} // namespace node
2 changes: 0 additions & 2 deletions src/node_errors.h
Original file line number Diff line number Diff line change
@@ -31,8 +31,6 @@ void FatalException(v8::Isolate* isolate,
v8::Local<v8::Value> error,
v8::Local<v8::Message> message);

void FatalException(const v8::FunctionCallbackInfo<v8::Value>& args);

// Helpers to construct errors similar to the ones provided by
// lib/internal/errors.js.
// Example: with `V(ERR_INVALID_ARG_TYPE, TypeError)`, there will be
25 changes: 25 additions & 0 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "env-inl.h"
#include "node.h"
#include "node_errors.h"
#include "node_internals.h"
#include "v8.h"

@@ -9,6 +10,7 @@ namespace node {

using v8::Array;
using v8::Context;
using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::Isolate;
@@ -17,6 +19,7 @@ using v8::kPromiseRejectAfterResolved;
using v8::kPromiseRejectWithNoHandler;
using v8::kPromiseResolveAfterResolved;
using v8::Local;
using v8::Message;
using v8::Number;
using v8::Object;
using v8::Promise;
@@ -26,6 +29,15 @@ using v8::Value;

namespace task_queue {

static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

CHECK(args[0]->IsFunction());

isolate->EnqueueMicrotask(args[0].As<Function>());
}

static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
args.GetIsolate()->RunMicrotasks();
}
@@ -95,13 +107,26 @@ static void SetPromiseRejectCallback(
env->set_promise_reject_callback(args[0].As<Function>());
}

static void TriggerFatalException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
if (env != nullptr && env->abort_on_uncaught_exception()) {
Abort();
}
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
FatalException(isolate, exception, message);
}

static void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();

env->SetMethod(target, "triggerFatalException", TriggerFatalException);
env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask);
env->SetMethod(target, "setTickCallback", SetTickCallback);
env->SetMethod(target, "runMicrotasks", RunMicrotasks);
target->Set(env->context(),
11 changes: 0 additions & 11 deletions src/node_util.cc
Original file line number Diff line number Diff line change
@@ -180,15 +180,6 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(args[0].As<ArrayBufferView>()->HasBuffer());
}

void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

CHECK(args[0]->IsFunction());

isolate->EnqueueMicrotask(args[0].As<Function>());
}

class WeakReference : public BaseObject {
public:
WeakReference(Environment* env, Local<Object> object, Local<Object> target)
@@ -261,8 +252,6 @@ void Initialize(Local<Object> target,
WatchdogHasPendingSigint);

env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask);
env->SetMethod(target, "triggerFatalException", FatalException);
Local<Object> constants = Object::New(env->isolate());
NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);
4 changes: 2 additions & 2 deletions test/message/events_unhandled_error_nexttick.out
Original file line number Diff line number Diff line change
@@ -13,7 +13,7 @@ Error
at internal/main/run_main_module.js:*:*
Emitted 'error' event at:
at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
at processTicksAndRejections (internal/process/next_tick.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
at processTicksAndRejections (internal/process/task_queues.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
4 changes: 2 additions & 2 deletions test/message/nexttick_throw.out
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@
^
ReferenceError: undefined_reference_error_maker is not defined
at *test*message*nexttick_throw.js:*:*
at processTicksAndRejections (internal/process/next_tick.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
at processTicksAndRejections (internal/process/task_queues.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*

0 comments on commit 8d669bb

Please sign in to comment.