Skip to content

Commit d2b972e

Browse files
bengldanielleadams
authored andcommitted
async_hooks: check for empty contexts before removing
This way we don't end up attempting to SetPromiseHooks on contexts that have already been collected. Fixes: #39019 PR-URL: #39095 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
1 parent 78d2e0e commit d2b972e

File tree

2 files changed

+26
-1
lines changed

2 files changed

+26
-1
lines changed

src/env-inl.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init,
104104
js_promise_hooks_[2].Reset(env()->isolate(), after);
105105
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
106106
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
107+
if (it->IsEmpty()) {
108+
it = contexts_.erase(it);
109+
it--;
110+
continue;
111+
}
107112
PersistentToLocal::Weak(env()->isolate(), *it)
108113
->SetPromiseHooks(init, before, after, resolve);
109114
}
@@ -256,8 +261,13 @@ inline void AsyncHooks::RemoveContext(v8::Local<v8::Context> ctx) {
256261
v8::Isolate* isolate = env()->isolate();
257262
v8::HandleScope handle_scope(isolate);
258263
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
264+
if (it->IsEmpty()) {
265+
it = contexts_.erase(it);
266+
it--;
267+
continue;
268+
}
259269
v8::Local<v8::Context> saved_context =
260-
PersistentToLocal::Weak(env()->isolate(), *it);
270+
PersistentToLocal::Weak(isolate, *it);
261271
if (saved_context == ctx) {
262272
it->Reset();
263273
contexts_.erase(it);
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Flags: --expose-gc
2+
'use strict';
3+
4+
require('../common');
5+
const asyncHooks = require('async_hooks');
6+
const vm = require('vm');
7+
8+
// This is a regression test for https://github.com/nodejs/node/issues/39019
9+
//
10+
// It should not segfault.
11+
12+
const hook = asyncHooks.createHook({ init() {} }).enable();
13+
vm.createContext();
14+
globalThis.gc();
15+
hook.disable();

0 commit comments

Comments
 (0)