Skip to content

Commit

Permalink
src: refactor tickInfo access
Browse files Browse the repository at this point in the history
- Wrap access to tickInfo fields in functions
- Rename `kHasScheduled` to `kHasTickScheduled` and
  `kHasPromiseRejections` to `kHasRejectionToWarn` for clarity - note
  the latter will be set to false if the rejection does not lead to
  a warning so the previous description is not accurate.
- Set `kHasRejectionToWarn` in JS land of relying on C++ to use
  an implict contract (return value of the promise rejection handler)
  to set it, as the decision is made entirely in JS land.
- Destructure promise reject event constants.

PR-URL: #25200
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and addaleax committed Jan 15, 2019
1 parent 5b1a987 commit abc4cc2
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 43 deletions.
22 changes: 15 additions & 7 deletions lib/internal/process/next_tick.js
Expand Up @@ -11,6 +11,8 @@ const {
} = internalBinding('task_queue');

const {
setHasRejectionToWarn,
hasRejectionToWarn,
promiseRejectHandler,
emitPromiseRejectionWarnings
} = require('internal/process/promises');
Expand All @@ -30,15 +32,21 @@ 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();
Expand Down Expand Up @@ -70,10 +78,10 @@ function internalTickCallback() {

emitAfter(asyncId);
}
tickInfo[kHasScheduled] = 0;
setHasTickScheduled(false);
runMicrotasks();
} while (!queue.isEmpty() || emitPromiseRejectionWarnings());
tickInfo[kHasPromiseRejections] = 0;
setHasRejectionToWarn(false);
}

class TickObject {
Expand Down Expand Up @@ -119,7 +127,7 @@ function nextTick(callback) {
}

if (queue.isEmpty())
tickInfo[kHasScheduled] = 1;
setHasTickScheduled(true);
queue.push(new TickObject(callback, args, getDefaultTriggerAsyncId()));
}

Expand Down
48 changes: 36 additions & 12 deletions lib/internal/process/promises.js
Expand Up @@ -2,24 +2,45 @@

const { safeToString } = internalBinding('util');
const {
promiseRejectEvents
tickInfo,
promiseRejectEvents: {
kPromiseRejectWithNoHandler,
kPromiseHandlerAddedAfterReject,
kPromiseResolveAfterResolved,
kPromiseRejectAfterResolved
}
} = 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;
}
}

Expand All @@ -38,7 +59,7 @@ function unhandledRejection(promise, reason) {
warned: false
});
pendingUnhandledRejections.push(promise);
return true;
setHasRejectionToWarn(true);
}

function handledRejection(promise) {
Expand All @@ -54,10 +75,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';
Expand Down Expand Up @@ -121,6 +143,8 @@ function emitPromiseRejectionWarnings() {
}

module.exports = {
hasRejectionToWarn,
setHasRejectionToWarn,
promiseRejectHandler,
emitPromiseRejectionWarnings
};
4 changes: 2 additions & 2 deletions src/callback_scope.cc
Expand Up @@ -95,7 +95,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();
}

Expand All @@ -106,7 +106,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;
}

Expand Down
12 changes: 4 additions & 8 deletions src/env-inl.h
Expand Up @@ -265,16 +265,12 @@ inline AliasedBuffer<uint8_t, v8::Uint8Array>& 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<v8::Context> context,
Expand Down
10 changes: 4 additions & 6 deletions src/env.h
Expand Up @@ -572,18 +572,16 @@ class Environment {
class TickInfo {
public:
inline AliasedBuffer<uint8_t, v8::Uint8Array>& 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
};

Expand Down
10 changes: 2 additions & 8 deletions src/node_task_queue.cc
Expand Up @@ -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;
Expand Down Expand Up @@ -80,13 +79,8 @@ static void PromiseRejectCallback(PromiseRejectMessage message) {
}

Local<Value> args[] = { type, promise, value };
MaybeLocal<Value> 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(
Expand Down

0 comments on commit abc4cc2

Please sign in to comment.