Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit cf0d7cf

Browse filesBrowse files
SebmasterMylesBorins
Sebmaster
authored and
MylesBorins
committed
async_hooks: add destroy event for gced AsyncResources
In cases where libraries create AsyncResources which may be emitting more events depending on usage, the only way to ensure that destroy is called properly is by calling it when the resource gets garbage collected. Fixes: #16153 PR-URL: #16998 Fixes: #16153 Reviewed-By: Andreas Madsen <amwebdk@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent a1a9957 commit cf0d7cf
Copy full SHA for cf0d7cf

File tree

Expand file treeCollapse file tree

10 files changed

+196
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

10 files changed

+196
-7
lines changed
Open diff view settings
Collapse file

‎benchmark/async_hooks/gc-tracking.js‎

Copy file name to clipboard
+45Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
const common = require('../common.js');
3+
const { AsyncResource } = require('async_hooks');
4+
5+
const bench = common.createBenchmark(main, {
6+
n: [1e6],
7+
method: [
8+
'trackingEnabled',
9+
'trackingDisabled',
10+
]
11+
}, {
12+
flags: ['--expose-gc']
13+
});
14+
15+
function endAfterGC(n) {
16+
setImmediate(() => {
17+
global.gc();
18+
setImmediate(() => {
19+
bench.end(n);
20+
});
21+
});
22+
}
23+
24+
function main(conf) {
25+
const n = +conf.n;
26+
27+
switch (conf.method) {
28+
case 'trackingEnabled':
29+
bench.start();
30+
for (let i = 0; i < n; i++) {
31+
new AsyncResource('foobar');
32+
}
33+
endAfterGC(n);
34+
break;
35+
case 'trackingDisabled':
36+
bench.start();
37+
for (let i = 0; i < n; i++) {
38+
new AsyncResource('foobar', { requireManualDestroy: true });
39+
}
40+
endAfterGC(n);
41+
break;
42+
default:
43+
throw new Error('Unsupported method');
44+
}
45+
}
Collapse file

‎doc/api/async_hooks.md‎

Copy file name to clipboardExpand all lines: doc/api/async_hooks.md
+13-5Lines changed: 13 additions & 5 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -543,12 +543,14 @@ will occur and the process will abort.
543543
The following is an overview of the `AsyncResource` API.
544544

545545
```js
546-
const { AsyncResource } = require('async_hooks');
546+
const { AsyncResource, executionAsyncId } = require('async_hooks');
547547

548548
// AsyncResource() is meant to be extended. Instantiating a
549549
// new AsyncResource() also triggers init. If triggerAsyncId is omitted then
550550
// async_hook.executionAsyncId() is used.
551-
const asyncResource = new AsyncResource(type, triggerAsyncId);
551+
const asyncResource = new AsyncResource(
552+
type, { triggerAsyncId: executionAsyncId(), requireManualDestroy: false }
553+
);
552554

553555
// Call AsyncHooks before callbacks.
554556
asyncResource.emitBefore();
@@ -566,11 +568,17 @@ asyncResource.asyncId();
566568
asyncResource.triggerAsyncId();
567569
```
568570

569-
#### `AsyncResource(type[, triggerAsyncId])`
571+
#### `AsyncResource(type[, options])`
570572

571573
* `type` {string} The type of async event.
572-
* `triggerAsyncId` {number} The ID of the execution context that created this
573-
async event.
574+
* `options` {Object}
575+
* `triggerAsyncId` {number} The ID of the execution context that created this
576+
async event. **Default:** `executionAsyncId()`
577+
* `requireManualDestroy` {boolean} Disables automatic `emitDestroy` when the
578+
object is garbage collected. This usually does not need to be set (even if
579+
`emitDestroy` is called manually), unless the resource's asyncId is retrieved
580+
and the sensitive API's `emitDestroy` is called with it.
581+
**Default:** `false`
574582

575583
Example usage:
576584

Collapse file

‎lib/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/async_hooks.js
+20-1Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ const { async_hook_fields, async_id_fields } = async_wrap;
2929
const { pushAsyncIds, popAsyncIds } = async_wrap;
3030
// For performance reasons, only track Proimses when a hook is enabled.
3131
const { enablePromiseHook, disablePromiseHook } = async_wrap;
32+
// For userland AsyncResources, make sure to emit a destroy event when the
33+
// resource gets gced.
34+
const { registerDestroyHook } = async_wrap;
3235
// Properties in active_hooks are used to keep track of the set of hooks being
3336
// executed in case another hook is enabled/disabled. The new set of hooks is
3437
// then restored once the active set of hooks is finished executing.
@@ -259,13 +262,22 @@ function validateAsyncId(asyncId, type) {
259262

260263
// Embedder API //
261264

265+
const destroyedSymbol = Symbol('destroyed');
266+
262267
class AsyncResource {
263-
constructor(type, triggerAsyncId = initTriggerId()) {
268+
constructor(type, opts = {}) {
264269
if (typeof type !== 'string')
265270
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'type', 'string');
266271

272+
if (typeof opts === 'number') {
273+
opts = { triggerAsyncId: opts, requireManualDestroy: false };
274+
} else if (opts.triggerAsyncId === undefined) {
275+
opts.triggerAsyncId = initTriggerId();
276+
}
277+
267278
// Unlike emitInitScript, AsyncResource doesn't supports null as the
268279
// triggerAsyncId.
280+
const triggerAsyncId = opts.triggerAsyncId;
269281
if (!Number.isSafeInteger(triggerAsyncId) || triggerAsyncId < -1) {
270282
throw new errors.RangeError('ERR_INVALID_ASYNC_ID',
271283
'triggerAsyncId',
@@ -274,10 +286,16 @@ class AsyncResource {
274286

275287
this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
276288
this[trigger_async_id_symbol] = triggerAsyncId;
289+
// this prop name (destroyed) has to be synchronized with C++
290+
this[destroyedSymbol] = { destroyed: false };
277291

278292
emitInitScript(
279293
this[async_id_symbol], type, this[trigger_async_id_symbol], this
280294
);
295+
296+
if (!opts.requireManualDestroy) {
297+
registerDestroyHook(this, this[async_id_symbol], this[destroyedSymbol]);
298+
}
281299
}
282300

283301
emitBefore() {
@@ -291,6 +309,7 @@ class AsyncResource {
291309
}
292310

293311
emitDestroy() {
312+
this[destroyedSymbol].destroyed = true;
294313
emitDestroyScript(this[async_id_symbol]);
295314
return this;
296315
}
Collapse file

‎src/async-wrap.cc‎

Copy file name to clipboardExpand all lines: src/async-wrap.cc
+41Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,46 @@ static void DisablePromiseHook(const FunctionCallbackInfo<Value>& args) {
427427
}
428428

429429

430+
class DestroyParam {
431+
public:
432+
double asyncId;
433+
v8::Persistent<Object> target;
434+
v8::Persistent<Object> propBag;
435+
};
436+
437+
438+
void AsyncWrap::WeakCallback(const v8::WeakCallbackInfo<DestroyParam>& info) {
439+
HandleScope scope(info.GetIsolate());
440+
441+
Environment* env = Environment::GetCurrent(info.GetIsolate());
442+
DestroyParam* p = info.GetParameter();
443+
Local<Object> prop_bag = PersistentToLocal(info.GetIsolate(), p->propBag);
444+
445+
Local<Value> val = prop_bag->Get(env->destroyed_string());
446+
if (val->IsFalse()) {
447+
AsyncWrap::EmitDestroy(env, p->asyncId);
448+
}
449+
p->target.Reset();
450+
p->propBag.Reset();
451+
delete p;
452+
}
453+
454+
455+
static void RegisterDestroyHook(const FunctionCallbackInfo<Value>& args) {
456+
CHECK(args[0]->IsObject());
457+
CHECK(args[1]->IsNumber());
458+
CHECK(args[2]->IsObject());
459+
460+
Isolate* isolate = args.GetIsolate();
461+
DestroyParam* p = new DestroyParam();
462+
p->asyncId = args[1].As<Number>()->Value();
463+
p->target.Reset(isolate, args[0].As<Object>());
464+
p->propBag.Reset(isolate, args[2].As<Object>());
465+
p->target.SetWeak(
466+
p, AsyncWrap::WeakCallback, v8::WeakCallbackType::kParameter);
467+
}
468+
469+
430470
void AsyncWrap::GetAsyncId(const FunctionCallbackInfo<Value>& args) {
431471
AsyncWrap* wrap;
432472
args.GetReturnValue().Set(-1);
@@ -502,6 +542,7 @@ void AsyncWrap::Initialize(Local<Object> target,
502542
env->SetMethod(target, "queueDestroyAsyncId", QueueDestroyAsyncId);
503543
env->SetMethod(target, "enablePromiseHook", EnablePromiseHook);
504544
env->SetMethod(target, "disablePromiseHook", DisablePromiseHook);
545+
env->SetMethod(target, "registerDestroyHook", RegisterDestroyHook);
505546

506547
v8::PropertyAttribute ReadOnlyDontDelete =
507548
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete);
Collapse file

‎src/async-wrap.h‎

Copy file name to clipboardExpand all lines: src/async-wrap.h
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ namespace node {
8585
NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
8686

8787
class Environment;
88+
class DestroyParam;
8889

8990
class AsyncWrap : public BaseObject {
9091
public:
@@ -164,6 +165,8 @@ class AsyncWrap : public BaseObject {
164165

165166
virtual size_t self_size() const = 0;
166167

168+
static void WeakCallback(const v8::WeakCallbackInfo<DestroyParam> &info);
169+
167170
private:
168171
friend class PromiseWrap;
169172

Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ class ModuleWrap;
120120
V(cwd_string, "cwd") \
121121
V(dest_string, "dest") \
122122
V(destroy_string, "destroy") \
123+
V(destroyed_string, "destroyed") \
123124
V(detached_string, "detached") \
124125
V(dns_a_string, "A") \
125126
V(dns_aaaa_string, "AAAA") \
Collapse file

‎test/async-hooks/test-embedder.api.async-resource.js‎

Copy file name to clipboardExpand all lines: test/async-hooks/test-embedder.api.async-resource.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ common.expectsError(
1818
type: TypeError,
1919
});
2020
assert.throws(() => {
21-
new AsyncResource('invalid_trigger_id', null);
21+
new AsyncResource('invalid_trigger_id', { triggerAsyncId: null });
2222
}, common.expectsError({
2323
code: 'ERR_INVALID_ASYNC_ID',
2424
type: RangeError,
Collapse file

‎test/parallel/test-async-hooks-destroy-on-gc.js‎

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
// Flags: --expose_gc
3+
4+
// This test ensures that userland-only AsyncResources cause a destroy event to
5+
// be emitted when they get gced.
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
const async_hooks = require('async_hooks');
10+
11+
const destroyedIds = new Set();
12+
async_hooks.createHook({
13+
destroy: common.mustCallAtLeast((asyncId) => {
14+
destroyedIds.add(asyncId);
15+
}, 1)
16+
}).enable();
17+
18+
let asyncId = null;
19+
{
20+
const res = new async_hooks.AsyncResource('foobar');
21+
asyncId = res.asyncId();
22+
}
23+
24+
setImmediate(() => {
25+
global.gc();
26+
setImmediate(() => assert.ok(destroyedIds.has(asyncId)));
27+
});
Collapse file

‎test/parallel/test-async-hooks-disable-gc-tracking.js‎

Copy file name to clipboard
+21Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
'use strict';
2+
// Flags: --expose_gc
3+
4+
// This test ensures that userland-only AsyncResources cause a destroy event to
5+
// be emitted when they get gced.
6+
7+
const common = require('../common');
8+
const async_hooks = require('async_hooks');
9+
10+
const hook = async_hooks.createHook({
11+
destroy: common.mustCall(1) // only 1 immediate is destroyed
12+
}).enable();
13+
14+
new async_hooks.AsyncResource('foobar', { requireManualDestroy: true });
15+
16+
setImmediate(() => {
17+
global.gc();
18+
setImmediate(() => {
19+
hook.disable();
20+
});
21+
});
Collapse file

‎test/parallel/test-async-hooks-prevent-double-destroy.js‎

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
// Flags: --expose_gc
3+
4+
// This test ensures that userland-only AsyncResources cause a destroy event to
5+
// be emitted when they get gced.
6+
7+
const common = require('../common');
8+
const async_hooks = require('async_hooks');
9+
10+
const hook = async_hooks.createHook({
11+
destroy: common.mustCall(2) // 1 immediate + manual destroy
12+
}).enable();
13+
14+
{
15+
const res = new async_hooks.AsyncResource('foobar');
16+
res.emitDestroy();
17+
}
18+
19+
setImmediate(() => {
20+
global.gc();
21+
setImmediate(() => {
22+
hook.disable();
23+
});
24+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.