domain: use strong reference to domain while active
When an uncaught exception is thrown inside a domain, the domain is removed from the stack as of 43a51708589ac789ce08beaeb49d6d778dfbdc49. This means that it might not be kept alive as an object anymore, and may be garbage collected before the `after()` hook can run, which tries to exit it as well. Resolve that by making references to the domain strong while it is active. Fixes: https://github.com/nodejs/node/issues/28275 PR-URL: https://github.com/nodejs/node/pull/28313 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
This commit is contained in:
parent
49eb2b8e2a
commit
43e5478e2f
@ -73,13 +73,18 @@ const asyncHook = createHook({
|
||||
if (current !== undefined) { // Enter domain for this cb
|
||||
// We will get the domain through current.get(), because the resource
|
||||
// object's .domain property makes sure it is not garbage collected.
|
||||
// However, we do need to make the reference to the domain non-weak,
|
||||
// so that it cannot be garbage collected before the after() hook.
|
||||
current.incRef();
|
||||
current.get().enter();
|
||||
}
|
||||
},
|
||||
after(asyncId) {
|
||||
const current = pairing.get(asyncId);
|
||||
if (current !== undefined) { // Exit domain for this cb
|
||||
current.get().exit();
|
||||
const domain = current.get();
|
||||
current.decRef();
|
||||
domain.exit();
|
||||
}
|
||||
},
|
||||
destroy(asyncId) {
|
||||
|
@ -189,12 +189,26 @@ class WeakReference : public BaseObject {
|
||||
args.GetReturnValue().Set(weak_ref->target_.Get(isolate));
|
||||
}
|
||||
|
||||
static void IncRef(const FunctionCallbackInfo<Value>& args) {
|
||||
WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
|
||||
if (weak_ref->reference_count_ == 0) weak_ref->target_.ClearWeak();
|
||||
weak_ref->reference_count_++;
|
||||
}
|
||||
|
||||
static void DecRef(const FunctionCallbackInfo<Value>& args) {
|
||||
WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
|
||||
CHECK_GE(weak_ref->reference_count_, 1);
|
||||
weak_ref->reference_count_--;
|
||||
if (weak_ref->reference_count_ == 0) weak_ref->target_.SetWeak();
|
||||
}
|
||||
|
||||
SET_MEMORY_INFO_NAME(WeakReference)
|
||||
SET_SELF_SIZE(WeakReference)
|
||||
SET_NO_MEMORY_INFO()
|
||||
|
||||
private:
|
||||
Global<Object> target_;
|
||||
uint64_t reference_count_ = 0;
|
||||
};
|
||||
|
||||
static void GuessHandleType(const FunctionCallbackInfo<Value>& args) {
|
||||
@ -294,6 +308,8 @@ void Initialize(Local<Object> target,
|
||||
weak_ref->InstanceTemplate()->SetInternalFieldCount(1);
|
||||
weak_ref->SetClassName(weak_ref_string);
|
||||
env->SetProtoMethod(weak_ref, "get", WeakReference::Get);
|
||||
env->SetProtoMethod(weak_ref, "incRef", WeakReference::IncRef);
|
||||
env->SetProtoMethod(weak_ref, "decRef", WeakReference::DecRef);
|
||||
target->Set(context, weak_ref_string,
|
||||
weak_ref->GetFunction(context).ToLocalChecked()).Check();
|
||||
|
||||
|
@ -1,3 +1,4 @@
|
||||
// Flags: --gc-interval=100 --stress-compaction
|
||||
'use strict';
|
||||
|
||||
const common = require('../common');
|
||||
@ -6,6 +7,8 @@ const domain = require('domain');
|
||||
|
||||
// This test is similar to test-domain-multiple-errors, but uses a new domain
|
||||
// for each errors.
|
||||
// The test flags are not essential, but serve as a way to verify that
|
||||
// https://github.com/nodejs/node/issues/28275 is fixed in debug mode.
|
||||
|
||||
for (const something of [
|
||||
42, null, undefined, false, () => {}, 'string', Symbol('foo')
|
||||
|
Loading…
x
Reference in New Issue
Block a user