Skip to content

Commit

Permalink
async_hooks: separate missing from default context
Browse files Browse the repository at this point in the history
When context is missing the executionAsyncId will be zero. For the
default triggerAsyncId the zero value was used to default to the
executionAsyncId. While this was not technically wrong because the
functions are different themself, it poorly separated the two concepts.

PR-URL: #17273
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
AndreasMadsen committed Dec 19, 2017
1 parent f3f1a93 commit 0784b04
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
6 changes: 3 additions & 3 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,9 @@ function getDefaultTriggerAsyncId() {
var defaultTriggerAsyncId = async_id_fields[kDefaultTriggerAsyncId];
// Reset value after it's been called so the next constructor doesn't
// inherit it by accident.
async_id_fields[kDefaultTriggerAsyncId] = 0;
async_id_fields[kDefaultTriggerAsyncId] = -1;
// If defaultTriggerAsyncId isn't set, use the executionAsyncId
if (defaultTriggerAsyncId <= 0)
if (defaultTriggerAsyncId < 0)
defaultTriggerAsyncId = async_id_fields[kExecutionAsyncId];
return defaultTriggerAsyncId;
}
Expand Down Expand Up @@ -288,7 +288,7 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) {
} else {
// If a triggerAsyncId was passed, any kDefaultTriggerAsyncId still must be
// null'd.
async_id_fields[kDefaultTriggerAsyncId] = 0;
async_id_fields[kDefaultTriggerAsyncId] = -1;
}

emitInitNative(asyncId, type, triggerAsyncId, resource);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@

// It's possible that kDefaultTriggerAsyncId was set for a constructor
// call that threw and was never cleared. So clear it now.
async_id_fields[kDefaultTriggerAsyncId] = 0;
async_id_fields[kDefaultTriggerAsyncId] = -1;

if (exceptionHandlerState.captureFn !== null) {
exceptionHandlerState.captureFn(er);
Expand Down
10 changes: 8 additions & 2 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate)
// and flag changes won't be included.
fields_[kCheck] = 1;

// kDefaultTriggerAsyncId should be -1, this indicates that there is no
// specified default value and it should fallback to the executionAsyncId.
// 0 is not used as the magic value, because that indicates a missing context
// which is different from a default context.
async_id_fields_[AsyncHooks::kDefaultTriggerAsyncId] = -1;

// kAsyncIdCounter should start at 1 because that'll be the id the execution
// context during bootstrap (code that runs before entering uv_run()).
async_id_fields_[AsyncHooks::kAsyncIdCounter] = 1;
Expand Down Expand Up @@ -450,9 +456,9 @@ inline double Environment::trigger_async_id() {
inline double Environment::get_default_trigger_async_id() {
double default_trigger_async_id =
async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId];
async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = 0;
async_hooks()->async_id_fields()[AsyncHooks::kDefaultTriggerAsyncId] = -1;
// If defaultTriggerAsyncId isn't set, use the executionAsyncId
if (default_trigger_async_id <= 0)
if (default_trigger_async_id < 0)
default_trigger_async_id = execution_async_id();
return default_trigger_async_id;
}
Expand Down

0 comments on commit 0784b04

Please sign in to comment.