Skip to content

Commit

Permalink
async_hooks: emitAfter correctly on fatalException
Browse files Browse the repository at this point in the history
Previously calling emitAfter() from _fatalException would skipt the
first asyncId. Instead use the size() of the std::stack to determine how
many times to loop and call emitAfter().

PR-URL: #14914
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
trevnorris authored and MylesBorins committed Sep 10, 2017
1 parent fa85f83 commit 44a5e61
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 3 deletions.
5 changes: 2 additions & 3 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@
// Arrays containing hook flags and ids for async_hook calls.
const { async_hook_fields, async_uid_fields } = async_wrap;
// Internal functions needed to manipulate the stack.
const { clearIdStack, popAsyncIds } = async_wrap;
const { clearIdStack, asyncIdStackSize } = async_wrap;
const { kAfter, kCurrentAsyncId, kInitTriggerId } = async_wrap.constants;

process._fatalException = function(er) {
Expand Down Expand Up @@ -384,8 +384,7 @@
do {
NativeModule.require('async_hooks').emitAfter(
async_uid_fields[kCurrentAsyncId]);
// popAsyncIds() returns true if there are more ids on the stack.
} while (popAsyncIds(async_uid_fields[kCurrentAsyncId]));
} while (asyncIdStackSize() > 0);
// Or completely empty the id stack.
} else {
clearIdStack();
Expand Down
8 changes: 8 additions & 0 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,13 @@ void AsyncWrap::PopAsyncIds(const FunctionCallbackInfo<Value>& args) {
}


void AsyncWrap::AsyncIdStackSize(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
args.GetReturnValue().Set(
static_cast<double>(env->async_hooks()->stack_size()));
}


void AsyncWrap::ClearIdStack(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
env->async_hooks()->clear_id_stack();
Expand Down Expand Up @@ -503,6 +510,7 @@ void AsyncWrap::Initialize(Local<Object> target,
env->SetMethod(target, "setupHooks", SetupHooks);
env->SetMethod(target, "pushAsyncIds", PushAsyncIds);
env->SetMethod(target, "popAsyncIds", PopAsyncIds);
env->SetMethod(target, "asyncIdStackSize", AsyncIdStackSize);
env->SetMethod(target, "clearIdStack", ClearIdStack);
env->SetMethod(target, "addIdToDestroyList", QueueDestroyId);
env->SetMethod(target, "enablePromiseHook", EnablePromiseHook);
Expand Down
1 change: 1 addition & 0 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class AsyncWrap : public BaseObject {
static void GetAsyncId(const v8::FunctionCallbackInfo<v8::Value>& args);
static void PushAsyncIds(const v8::FunctionCallbackInfo<v8::Value>& args);
static void PopAsyncIds(const v8::FunctionCallbackInfo<v8::Value>& args);
static void AsyncIdStackSize(const v8::FunctionCallbackInfo<v8::Value>& args);
static void ClearIdStack(const v8::FunctionCallbackInfo<v8::Value>& args);
static void AsyncReset(const v8::FunctionCallbackInfo<v8::Value>& args);
static void QueueDestroyId(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
4 changes: 4 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ inline bool Environment::AsyncHooks::pop_ids(double async_id) {
return !ids_stack_.empty();
}

inline size_t Environment::AsyncHooks::stack_size() {
return ids_stack_.size();
}

inline void Environment::AsyncHooks::clear_id_stack() {
while (!ids_stack_.empty())
ids_stack_.pop();
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ class Environment {

inline void push_ids(double async_id, double trigger_id);
inline bool pop_ids(double async_id);
inline size_t stack_size();
inline void clear_id_stack(); // Used in fatal exceptions.

// Used to propagate the trigger_id to the constructor of any newly created
Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-emit-after-uncaught-exception.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');

const id_obj = {};
let collect = true;

const hook = async_hooks.createHook({
before(id) { if (collect) id_obj[id] = true; },
after(id) { delete id_obj[id]; },
}).enable();

process.once('uncaughtException', common.mustCall((er) => {
assert.strictEqual(er.message, 'bye');
collect = false;
}));

setImmediate(common.mustCall(() => {
process.nextTick(common.mustCall(() => {
assert.strictEqual(Object.keys(id_obj).length, 0);
hook.disable();
}));

// Create a stack of async ids that will need to be emitted in the case of
// an uncaught exception.
const ar1 = new async_hooks.AsyncResource('Mine');
ar1.emitBefore();

const ar2 = new async_hooks.AsyncResource('Mine');
ar2.emitBefore();

throw new Error('bye');

// TODO(trevnorris): This test shows that the after() hooks are always called
// correctly, but it doesn't solve where the emitDestroy() is missed because
// of the uncaught exception. Simple solution is to always call emitDestroy()
// before the emitAfter(), but how to codify this?
}));

0 comments on commit 44a5e61

Please sign in to comment.