From b2b33f3aac63aafbf0fafaa9ef01cd7a9354882d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 8 Mar 2019 16:28:19 +0100 Subject: [PATCH] process: refactor global.queueMicrotask() - 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()` --- lib/internal/bootstrap/node.js | 19 ++---- lib/internal/modules/cjs/loader.js | 2 +- .../process/{next_tick.js => task_queues.js} | 68 +++++++++++++++---- lib/internal/queue_microtask.js | 38 ----------- node.gyp | 3 +- src/node_errors.cc | 12 ---- src/node_errors.h | 2 - src/node_task_queue.cc | 25 +++++++ src/node_util.cc | 11 --- .../events_unhandled_error_nexttick.out | 4 +- test/message/nexttick_throw.out | 4 +- 11 files changed, 92 insertions(+), 96 deletions(-) rename lib/internal/process/{next_tick.js => task_queues.js} (72%) delete mode 100644 lib/internal/queue_microtask.js diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 83e1507441be28..d3a18b4d0fb139 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -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'); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 83ab464b40fd9c..ab44324b94e558 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -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); diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/task_queues.js similarity index 72% rename from lib/internal/process/next_tick.js rename to lib/internal/process/task_queues.js index 24104ec34abaee..9e4fba6d9df4f0 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/task_queues.js @@ -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 }; diff --git a/lib/internal/queue_microtask.js b/lib/internal/queue_microtask.js deleted file mode 100644 index 6ca0c1e144a92b..00000000000000 --- a/lib/internal/queue_microtask.js +++ /dev/null @@ -1,38 +0,0 @@ -'use strict'; - -const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes; -const { AsyncResource } = require('async_hooks'); -const { getDefaultTriggerAsyncId } = require('internal/async_hooks'); -const { - enqueueMicrotask, - triggerFatalException -} = internalBinding('util'); - -function queueMicrotask(callback) { - if (typeof callback !== 'function') { - throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback); - } - - const asyncResource = new AsyncResource('Microtask', { - triggerAsyncId: getDefaultTriggerAsyncId(), - requireManualDestroy: true, - }); - - 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 = { queueMicrotask }; diff --git a/node.gyp b/node.gyp index 64d0721d249705..9903e26048ba37 100644 --- a/node.gyp +++ b/node.gyp @@ -160,7 +160,6 @@ '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', @@ -168,8 +167,8 @@ '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', diff --git a/src/node_errors.cc b/src/node_errors.cc index 5c1e227966c113..1923cb6a22f427 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -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& args) { - Isolate* isolate = args.GetIsolate(); - Environment* env = Environment::GetCurrent(isolate); - if (env != nullptr && env->abort_on_uncaught_exception()) { - Abort(); - } - Local exception = args[0]; - Local message = Exception::CreateMessage(isolate, exception); - FatalException(isolate, exception, message); -} - } // namespace node diff --git a/src/node_errors.h b/src/node_errors.h index 28bf6a670f8ecc..d37dd7106ba744 100644 --- a/src/node_errors.h +++ b/src/node_errors.h @@ -31,8 +31,6 @@ void FatalException(v8::Isolate* isolate, v8::Local error, v8::Local message); -void FatalException(const v8::FunctionCallbackInfo& 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 diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index b7bafe6db69c36..64709f531d71c6 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -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& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + CHECK(args[0]->IsFunction()); + + isolate->EnqueueMicrotask(args[0].As()); +} + static void RunMicrotasks(const FunctionCallbackInfo& args) { args.GetIsolate()->RunMicrotasks(); } @@ -95,6 +107,17 @@ static void SetPromiseRejectCallback( env->set_promise_reject_callback(args[0].As()); } +static void TriggerFatalException(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + Environment* env = Environment::GetCurrent(isolate); + if (env != nullptr && env->abort_on_uncaught_exception()) { + Abort(); + } + Local exception = args[0]; + Local message = Exception::CreateMessage(isolate, exception); + FatalException(isolate, exception, message); +} + static void Initialize(Local target, Local unused, Local context, @@ -102,6 +125,8 @@ static void Initialize(Local target, 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(), diff --git a/src/node_util.cc b/src/node_util.cc index 6d20f636f0d25e..b1cdc11a998ea3 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -180,15 +180,6 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(args[0].As()->HasBuffer()); } -void EnqueueMicrotask(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); - - CHECK(args[0]->IsFunction()); - - isolate->EnqueueMicrotask(args[0].As()); -} - class WeakReference : public BaseObject { public: WeakReference(Environment* env, Local object, Local target) @@ -261,8 +252,6 @@ void Initialize(Local target, WatchdogHasPendingSigint); env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer); - env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask); - env->SetMethod(target, "triggerFatalException", FatalException); Local constants = Object::New(env->isolate()); NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES); NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE); diff --git a/test/message/events_unhandled_error_nexttick.out b/test/message/events_unhandled_error_nexttick.out index 4132ae9f3bc1be..fba05879fcc003 100644 --- a/test/message/events_unhandled_error_nexttick.out +++ b/test/message/events_unhandled_error_nexttick.out @@ -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:*:* diff --git a/test/message/nexttick_throw.out b/test/message/nexttick_throw.out index 7aa38a0424bd99..f180c4ab55a7c0 100644 --- a/test/message/nexttick_throw.out +++ b/test/message/nexttick_throw.out @@ -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:*:*