Skip to content

Commit

Permalink
async_hooks: run destroy cbs before normal exit
Browse files Browse the repository at this point in the history
Run destroy callbacks before a normal application exit happens.

Fixes: nodejs#13262
  • Loading branch information
addaleax committed May 29, 2017
1 parent 1091327 commit fb88c0d
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,13 @@ static void DestroyIdsCb(uv_idle_t* handle) {
uv_idle_stop(handle);

Environment* env = Environment::from_destroy_ids_idle_handle(handle);

HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());

AsyncWrap::RunDestroyCbs(env);
}

void AsyncWrap::RunDestroyCbs(Environment* env) {
Local<Function> fn = env->async_hooks_destroy_function();

TryCatch try_catch(env->isolate());
Expand All @@ -164,8 +168,6 @@ static void DestroyIdsCb(uv_idle_t* handle) {
FatalException(env->isolate(), try_catch);
}
}

env->destroy_ids_list()->clear();
}


Expand Down
2 changes: 2 additions & 0 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ class AsyncWrap : public BaseObject {
static bool EmitBefore(Environment* env, double id);
static bool EmitAfter(Environment* env, double id);

static void RunDestroyCbs(Environment* env);

inline ProviderType provider_type() const;

inline double get_id() const;
Expand Down
2 changes: 2 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4531,6 +4531,8 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
} while (more == true);
}

AsyncWrap::RunDestroyCbs(&env);

env.set_trace_sync_io(false);

const int exit_code = EmitExit(&env);
Expand Down
38 changes: 38 additions & 0 deletions test/parallel/test-async-hooks-close-during-destroy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';
// Test that async ids that are added to the destroy queue while running a
// `destroy` callback are handled correctly.

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

const initCalls = new Set();
let destroyTcpWrapCallCount = 0;
let srv2;

async_hooks.createHook({
init: common.mustCallAtLeast((id, provider, triggerId) => {
if (provider === 'TCPWRAP')
initCalls.add(id);
}, 2),
destroy: common.mustCallAtLeast((id) => {
if (!initCalls.has(id)) return;

switch (destroyTcpWrapCallCount++) {
case 0:
// Trigger the second `destroy` call.
srv2.close();
break;
case 2:
assert.fail('More than 2 destroy() invocations');
break;
}
}, 2)
}).enable();

// Create a server to trigger the first `destroy` callback.
net.createServer().listen(0).close();
srv2 = net.createServer().listen(0);

process.on('exit', () => assert.strictEqual(destroyTcpWrapCallCount, 2));
27 changes: 27 additions & 0 deletions test/parallel/test-async-hooks-top-level-clearimmediate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

// Regression test for https://github.com/nodejs/node/issues/13262

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

let seenId, seenResource;

async_hooks.createHook({
init: common.mustCall((id, provider, triggerId, resource) => {
seenId = id;
seenResource = resource;
assert.strictEqual(provider, 'Immediate');
assert.strictEqual(triggerId, 1);
}),
before: common.mustNotCall(),
after: common.mustNotCall(),
destroy: common.mustCall((id) => {
assert.strictEqual(seenId, id);
})
}).enable();

const immediate = setImmediate(common.mustNotCall());
assert.strictEqual(immediate, seenResource);
clearImmediate(immediate);

0 comments on commit fb88c0d

Please sign in to comment.