FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

async_hooks: add destroy event for gced AsyncResources · nodejs/node@cf0d7cf · GitHub

/ node Public

Commit cf0d7cf

Browse files
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

10 files changed

Lines changed: 196 additions & 7 deletions

‎benchmark/async_hooks/gc-tracking.js‎

Lines 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+
}

‎doc/api/async_hooks.md‎

Lines changed: 13 additions & 5 deletions
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

‎lib/async_hooks.js‎

Lines 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
}

‎src/async-wrap.cc‎

Lines 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);

‎src/async-wrap.h‎

Lines 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

‎src/env.h‎

Lines 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") \

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

Lines 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,

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

Lines 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+
});

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

Lines 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+
});

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

Lines 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)
Back | FazBrowse Home | New Git URL