diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 00bba254718684..24104ec34abaee 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -6,13 +6,14 @@ const { tickInfo, // Used to run V8's micro task queue. runMicrotasks, - setTickCallback, - initializePromiseRejectCallback + setTickCallback } = internalBinding('task_queue'); const { - promiseRejectHandler, - emitPromiseRejectionWarnings + setHasRejectionToWarn, + hasRejectionToWarn, + listenForRejections, + processPromiseRejections } = require('internal/process/promises'); const { @@ -30,21 +31,27 @@ const { ERR_INVALID_CALLBACK } = require('internal/errors').codes; const FixedQueue = require('internal/fixed_queue'); // *Must* match Environment::TickInfo::Fields in src/env.h. -const kHasScheduled = 0; -const kHasPromiseRejections = 1; +const kHasTickScheduled = 0; + +function hasTickScheduled() { + return tickInfo[kHasTickScheduled] === 1; +} +function setHasTickScheduled(value) { + tickInfo[kHasTickScheduled] = value ? 1 : 0; +} const queue = new FixedQueue(); function runNextTicks() { - if (tickInfo[kHasScheduled] === 0 && tickInfo[kHasPromiseRejections] === 0) + if (!hasTickScheduled() && !hasRejectionToWarn()) runMicrotasks(); - if (tickInfo[kHasScheduled] === 0 && tickInfo[kHasPromiseRejections] === 0) + if (!hasTickScheduled() && !hasRejectionToWarn()) return; - internalTickCallback(); + processTicksAndRejections(); } -function internalTickCallback() { +function processTicksAndRejections() { let tock; do { while (tock = queue.shift()) { @@ -70,10 +77,10 @@ function internalTickCallback() { emitAfter(asyncId); } - tickInfo[kHasScheduled] = 0; + setHasTickScheduled(false); runMicrotasks(); - } while (!queue.isEmpty() || emitPromiseRejectionWarnings()); - tickInfo[kHasPromiseRejections] = 0; + } while (!queue.isEmpty() || processPromiseRejections()); + setHasRejectionToWarn(false); } class TickObject { @@ -119,18 +126,17 @@ function nextTick(callback) { } if (queue.isEmpty()) - tickInfo[kHasScheduled] = 1; + setHasTickScheduled(true); 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() { - // Initializes the per-isolate promise rejection callback which - // will call the handler being passed into this function. - initializePromiseRejectCallback(promiseRejectHandler); + // Sets the per-isolate promise rejection callback + listenForRejections(); // Sets the callback to be run in every tick. - setTickCallback(internalTickCallback); + setTickCallback(processTicksAndRejections); return { nextTick, runNextTicks diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index df4e8188fe92b4..f3e118e2ce7509 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -2,24 +2,46 @@ const { safeToString } = internalBinding('util'); const { - promiseRejectEvents + tickInfo, + promiseRejectEvents: { + kPromiseRejectWithNoHandler, + kPromiseHandlerAddedAfterReject, + kPromiseResolveAfterResolved, + kPromiseRejectAfterResolved + }, + setPromiseRejectCallback } = internalBinding('task_queue'); +// *Must* match Environment::TickInfo::Fields in src/env.h. +const kHasRejectionToWarn = 1; + const maybeUnhandledPromises = new WeakMap(); const pendingUnhandledRejections = []; const asyncHandledRejections = []; let lastPromiseId = 0; +function setHasRejectionToWarn(value) { + tickInfo[kHasRejectionToWarn] = value ? 1 : 0; +} + +function hasRejectionToWarn() { + return tickInfo[kHasRejectionToWarn] === 1; +} + function promiseRejectHandler(type, promise, reason) { switch (type) { - case promiseRejectEvents.kPromiseRejectWithNoHandler: - return unhandledRejection(promise, reason); - case promiseRejectEvents.kPromiseHandlerAddedAfterReject: - return handledRejection(promise); - case promiseRejectEvents.kPromiseResolveAfterResolved: - return resolveError('resolve', promise, reason); - case promiseRejectEvents.kPromiseRejectAfterResolved: - return resolveError('reject', promise, reason); + case kPromiseRejectWithNoHandler: + unhandledRejection(promise, reason); + break; + case kPromiseHandlerAddedAfterReject: + handledRejection(promise); + break; + case kPromiseResolveAfterResolved: + resolveError('resolve', promise, reason); + break; + case kPromiseRejectAfterResolved: + resolveError('reject', promise, reason); + break; } } @@ -38,7 +60,7 @@ function unhandledRejection(promise, reason) { warned: false }); pendingUnhandledRejections.push(promise); - return true; + setHasRejectionToWarn(true); } function handledRejection(promise) { @@ -54,10 +76,11 @@ function handledRejection(promise) { warning.name = 'PromiseRejectionHandledWarning'; warning.id = uid; asyncHandledRejections.push({ promise, warning }); - return true; + setHasRejectionToWarn(true); + return; } } - return false; + setHasRejectionToWarn(false); } const unhandledRejectionErrName = 'UnhandledPromiseRejectionWarning'; @@ -95,7 +118,9 @@ function emitDeprecationWarning() { } } -function emitPromiseRejectionWarnings() { +// If this method returns true, at least one more tick need to be +// scheduled to process any potential pending rejections +function processPromiseRejections() { while (asyncHandledRejections.length > 0) { const { promise, warning } = asyncHandledRejections.shift(); if (!process.emit('rejectionHandled', promise)) { @@ -120,7 +145,13 @@ function emitPromiseRejectionWarnings() { return maybeScheduledTicks || pendingUnhandledRejections.length !== 0; } +function listenForRejections() { + setPromiseRejectCallback(promiseRejectHandler); +} + module.exports = { - promiseRejectHandler, - emitPromiseRejectionWarnings + hasRejectionToWarn, + setHasRejectionToWarn, + listenForRejections, + processPromiseRejections }; diff --git a/src/callback_scope.cc b/src/callback_scope.cc index 0757b61061c3e7..fee7417fd37187 100644 --- a/src/callback_scope.cc +++ b/src/callback_scope.cc @@ -5,6 +5,7 @@ namespace node { +using v8::Function; using v8::HandleScope; using v8::Isolate; using v8::Local; @@ -95,7 +96,7 @@ void InternalCallbackScope::Close() { Environment::TickInfo* tick_info = env_->tick_info(); if (!env_->can_call_into_js()) return; - if (!tick_info->has_scheduled()) { + if (!tick_info->has_tick_scheduled()) { env_->isolate()->RunMicrotasks(); } @@ -106,7 +107,7 @@ void InternalCallbackScope::Close() { CHECK_EQ(env_->trigger_async_id(), 0); } - if (!tick_info->has_scheduled() && !tick_info->has_promise_rejections()) { + if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn()) { return; } @@ -114,8 +115,13 @@ void InternalCallbackScope::Close() { if (!env_->can_call_into_js()) return; - if (env_->tick_callback_function() - ->Call(env_->context(), process, 0, nullptr).IsEmpty()) { + Local tick_callback = env_->tick_callback_function(); + + // The tick is triggered before JS land calls SetTickCallback + // to initializes the tick callback during bootstrap. + CHECK(!tick_callback.IsEmpty()); + + if (tick_callback->Call(env_->context(), process, 0, nullptr).IsEmpty()) { failed_ = true; } } diff --git a/src/env-inl.h b/src/env-inl.h index e323d8707e676f..e63d0d38aa2aea 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -265,16 +265,12 @@ inline AliasedBuffer& Environment::TickInfo::fields() { return fields_; } -inline bool Environment::TickInfo::has_scheduled() const { - return fields_[kHasScheduled] == 1; +inline bool Environment::TickInfo::has_tick_scheduled() const { + return fields_[kHasTickScheduled] == 1; } -inline bool Environment::TickInfo::has_promise_rejections() const { - return fields_[kHasPromiseRejections] == 1; -} - -inline void Environment::TickInfo::promise_rejections_toggle_on() { - fields_[kHasPromiseRejections] = 1; +inline bool Environment::TickInfo::has_rejection_to_warn() const { + return fields_[kHasRejectionToWarn] == 1; } inline void Environment::AssignToContext(v8::Local context, diff --git a/src/env.cc b/src/env.cc index d8c4dce2058b03..3d06ebe2082b08 100644 --- a/src/env.cc +++ b/src/env.cc @@ -241,6 +241,10 @@ Environment::Environment(IsolateData* isolate_data, if (options_->no_force_async_hooks_checks) { async_hooks_.no_force_checks(); } + + // TODO(addaleax): the per-isolate state should not be controlled by + // a single Environment. + isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback); } Environment::~Environment() { diff --git a/src/env.h b/src/env.h index fbb64f48b537fa..a55bcc53b9a502 100644 --- a/src/env.h +++ b/src/env.h @@ -574,18 +574,16 @@ class Environment { class TickInfo { public: inline AliasedBuffer& fields(); - inline bool has_scheduled() const; - inline bool has_promise_rejections() const; - - inline void promise_rejections_toggle_on(); + inline bool has_tick_scheduled() const; + inline bool has_rejection_to_warn() const; private: friend class Environment; // So we can call the constructor. inline explicit TickInfo(v8::Isolate* isolate); enum Fields { - kHasScheduled, - kHasPromiseRejections, + kHasTickScheduled = 0, + kHasRejectionToWarn, kFieldsCount }; diff --git a/src/node_internals.h b/src/node_internals.h index 897905b5a76a69..9b68474f7d44a7 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -180,6 +180,10 @@ SlicedArguments::SlicedArguments( size_ = size; } +namespace task_queue { +void PromiseRejectCallback(v8::PromiseRejectMessage message); +} // namespace task_queue + v8::Maybe ProcessEmitWarning(Environment* env, const char* fmt, ...); v8::Maybe ProcessEmitDeprecationWarning(Environment* env, const char* warning, diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index f65f420081d6e8..43d8e1cc246b8c 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -17,7 +17,6 @@ using v8::kPromiseRejectAfterResolved; using v8::kPromiseRejectWithNoHandler; using v8::kPromiseResolveAfterResolved; using v8::Local; -using v8::MaybeLocal; using v8::Number; using v8::Object; using v8::Promise; @@ -37,7 +36,7 @@ static void SetTickCallback(const FunctionCallbackInfo& args) { env->set_tick_callback_function(args[0].As()); } -static void PromiseRejectCallback(PromiseRejectMessage message) { +void PromiseRejectCallback(PromiseRejectMessage message) { static std::atomic unhandledRejections{0}; static std::atomic rejectionsHandledAfter{0}; @@ -50,6 +49,10 @@ static void PromiseRejectCallback(PromiseRejectMessage message) { if (env == nullptr) return; Local callback = env->promise_reject_callback(); + // The promise is rejected before JS land calls SetPromiseRejectCallback + // to initializes the promise reject callback during bootstrap. + CHECK(!callback.IsEmpty()); + Local value; Local type = Number::New(env->isolate(), event); @@ -80,26 +83,16 @@ static void PromiseRejectCallback(PromiseRejectMessage message) { } Local args[] = { type, promise, value }; - MaybeLocal ret = callback->Call(env->context(), - Undefined(isolate), - arraysize(args), - args); - - if (!ret.IsEmpty() && ret.ToLocalChecked()->IsTrue()) - env->tick_info()->promise_rejections_toggle_on(); + USE(callback->Call( + env->context(), Undefined(isolate), arraysize(args), args)); } -static void InitializePromiseRejectCallback( +static void SetPromiseRejectCallback( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); CHECK(args[0]->IsFunction()); - - // TODO(joyeecheung): this may be moved to somewhere earlier in the bootstrap - // to make sure it's only called once - isolate->SetPromiseRejectCallback(PromiseRejectCallback); - env->set_promise_reject_callback(args[0].As()); } @@ -126,8 +119,8 @@ static void Initialize(Local target, FIXED_ONE_BYTE_STRING(isolate, "promiseRejectEvents"), events).FromJust(); env->SetMethod(target, - "initializePromiseRejectCallback", - InitializePromiseRejectCallback); + "setPromiseRejectCallback", + SetPromiseRejectCallback); } } // namespace task_queue diff --git a/test/message/events_unhandled_error_nexttick.out b/test/message/events_unhandled_error_nexttick.out index aa52367ba09499..25575baa2c9ff0 100644 --- a/test/message/events_unhandled_error_nexttick.out +++ b/test/message/events_unhandled_error_nexttick.out @@ -15,7 +15,7 @@ Error at startup (internal/bootstrap/node.js:*:*) Emitted 'error' event at: at process.nextTick (*events_unhandled_error_nexttick.js:*:*) - at internalTickCallback (internal/process/next_tick.js:*:*) + at processTicksAndRejections (internal/process/next_tick.js:*:*) at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*) at Function.Module.runMain (internal/modules/cjs/loader.js:*:*) at executeUserCode (internal/bootstrap/node.js:*:*) diff --git a/test/message/nexttick_throw.out b/test/message/nexttick_throw.out index 1fee9075fe5078..aedecb169f3ea5 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 internalTickCallback (internal/process/next_tick.js:*:*) + at processTicksAndRejections (internal/process/next_tick.js:*:*) at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*) at Function.Module.runMain (internal/modules/cjs/loader.js:*:*) at executeUserCode (internal/bootstrap/node.js:*:*) diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index ac1a4df02816df..2bf935d7cbd16f 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -12,7 +12,7 @@ SyntaxError: Strict mode code may not include a with statement at Socket.process.stdin.on (internal/bootstrap/node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at process.internalTickCallback (internal/process/next_tick.js:*:*) + at processTicksAndRejections (internal/process/next_tick.js:*:*) 42 42 [stdin]:1 @@ -29,7 +29,7 @@ Error: hello at Socket.process.stdin.on (internal/bootstrap/node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at process.internalTickCallback (internal/process/next_tick.js:*:*) + at processTicksAndRejections (internal/process/next_tick.js:*:*) [stdin]:1 throw new Error("hello") ^ @@ -44,7 +44,7 @@ Error: hello at Socket.process.stdin.on (internal/bootstrap/node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at process.internalTickCallback (internal/process/next_tick.js:*:*) + at processTicksAndRejections (internal/process/next_tick.js:*:*) 100 [stdin]:1 var x = 100; y = x; @@ -60,7 +60,7 @@ ReferenceError: y is not defined at Socket.process.stdin.on (internal/bootstrap/node.js:*:*) at Socket.emit (events.js:*:*) at endReadableNT (_stream_readable.js:*:*) - at process.internalTickCallback (internal/process/next_tick.js:*:*) + at processTicksAndRejections (internal/process/next_tick.js:*:*) [stdin]:1 var ______________________________________________; throw 10