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: https://github.com/nodejs/node/issues/39019

PR-URL: https://github.com/nodejs/node/pull/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>
This commit is contained in:
Bryan English
2021-06-19 13:47:43 -04:00
parent 05dfc25d81
commit 0e8bd79cbb
2 changed files with 26 additions and 1 deletions

View File

@@ -104,6 +104,11 @@ inline void AsyncHooks::SetJSPromiseHooks(v8::Local<v8::Function> init,
js_promise_hooks_[2].Reset(env()->isolate(), after);
js_promise_hooks_[3].Reset(env()->isolate(), resolve);
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
if (it->IsEmpty()) {
it = contexts_.erase(it);
it--;
continue;
}
PersistentToLocal::Weak(env()->isolate(), *it)
->SetPromiseHooks(init, before, after, resolve);
}
@@ -256,8 +261,13 @@ inline void AsyncHooks::RemoveContext(v8::Local<v8::Context> ctx) {
v8::Isolate* isolate = env()->isolate();
v8::HandleScope handle_scope(isolate);
for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
if (it->IsEmpty()) {
it = contexts_.erase(it);
it--;
continue;
}
v8::Local<v8::Context> saved_context =
PersistentToLocal::Weak(env()->isolate(), *it);
PersistentToLocal::Weak(isolate, *it);
if (saved_context == ctx) {
it->Reset();
contexts_.erase(it);

View File

@@ -0,0 +1,15 @@
// Flags: --expose-gc
'use strict';
require('../common');
const asyncHooks = require('async_hooks');
const vm = require('vm');
// This is a regression test for https://github.com/nodejs/node/issues/39019
//
// It should not segfault.
const hook = asyncHooks.createHook({ init() {} }).enable();
vm.createContext();
globalThis.gc();
hook.disable();