Skip to content

Commit 4eac937

Browse files
authored
src: release context frame in AsyncWrap::EmitDestroy
Release the async context frame in AsyncWrap::EmitDestroy to allow gc to collect it. This is in special relevant for reused resources like HTTPParser otherwise they might keep ALS stores alive. PR-URL: #61995 Fixes: #61882 Refs: #48528 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent 2422ed8 commit 4eac937

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

src/async_wrap.cc

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
#include "env-inl.h"
2626
#include "node_errors.h"
2727
#include "node_external_reference.h"
28+
#ifdef DEBUG
29+
#include <node_process-inl.h>
30+
#endif
2831
#include "tracing/traced_value.h"
2932
#include "util-inl.h"
3033

@@ -333,9 +336,12 @@ void AsyncWrap::EmitDestroy(bool from_gc) {
333336
// Ensure no double destroy is emitted via AsyncReset().
334337
async_id_ = kInvalidAsyncId;
335338

336-
if (!persistent().IsEmpty() && !from_gc) {
337-
HandleScope handle_scope(env()->isolate());
338-
USE(object()->Set(env()->context(), env()->resource_symbol(), object()));
339+
if (!from_gc) {
340+
if (!persistent().IsEmpty()) {
341+
HandleScope handle_scope(env()->isolate());
342+
USE(object()->Set(env()->context(), env()->resource_symbol(), object()));
343+
}
344+
context_frame_.Reset();
339345
}
340346
}
341347

@@ -659,6 +665,14 @@ MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
659665
Local<Value>* argv) {
660666
EmitTraceEventBefore();
661667

668+
#ifdef DEBUG
669+
if (context_frame_.IsEmpty()) {
670+
ProcessEmitWarning(env(),
671+
"MakeCallback() called without context_frame, "
672+
"likely use after destroy of AsyncWrap.");
673+
}
674+
#endif
675+
662676
ProviderType provider = provider_type();
663677
async_context context { get_async_id(), get_trigger_async_id() };
664678
MaybeLocal<Value> ret =
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Flags: --expose-gc
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { onGC } = require('../common/gc');
6+
const assert = require('node:assert');
7+
const { AsyncLocalStorage } = require('node:async_hooks');
8+
const { freeParser, parsers, HTTPParser } = require('_http_common');
9+
10+
let storeGCed = false;
11+
12+
const als = new AsyncLocalStorage();
13+
14+
function test() {
15+
const store = {};
16+
onGC(store, { ongc: common.mustCall(() => { storeGCed = true; }) });
17+
let parser;
18+
als.run(store, common.mustCall(() => {
19+
parser = parsers.alloc();
20+
parser.initialize(HTTPParser.RESPONSE, {});
21+
}));
22+
freeParser(parser);
23+
}
24+
25+
test();
26+
global.gc();
27+
setImmediate(common.mustCall(() => {
28+
assert.ok(storeGCed);
29+
}));

0 commit comments

Comments
 (0)