Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 727ffe4

Browse filesBrowse files
addaleaxtargos
authored andcommitted
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 43a5170. 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: #28275 PR-URL: #28313 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent ae56a23 commit 727ffe4
Copy full SHA for 727ffe4

File tree

Expand file treeCollapse file tree

3 files changed

+25
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+25
-1
lines changed
Open diff view settings
Collapse file

‎lib/domain.js‎

Copy file name to clipboardExpand all lines: lib/domain.js
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,18 @@ const asyncHook = createHook({
7373
if (current !== undefined) { // Enter domain for this cb
7474
// We will get the domain through current.get(), because the resource
7575
// object's .domain property makes sure it is not garbage collected.
76+
// However, we do need to make the reference to the domain non-weak,
77+
// so that it cannot be garbage collected before the after() hook.
78+
current.incRef();
7679
current.get().enter();
7780
}
7881
},
7982
after(asyncId) {
8083
const current = pairing.get(asyncId);
8184
if (current !== undefined) { // Exit domain for this cb
82-
current.get().exit();
85+
const domain = current.get();
86+
current.decRef();
87+
domain.exit();
8388
}
8489
},
8590
destroy(asyncId) {
Collapse file

‎src/node_util.cc‎

Copy file name to clipboardExpand all lines: src/node_util.cc
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,26 @@ class WeakReference : public BaseObject {
189189
args.GetReturnValue().Set(weak_ref->target_.Get(isolate));
190190
}
191191

192+
static void IncRef(const FunctionCallbackInfo<Value>& args) {
193+
WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
194+
if (weak_ref->reference_count_ == 0) weak_ref->target_.ClearWeak();
195+
weak_ref->reference_count_++;
196+
}
197+
198+
static void DecRef(const FunctionCallbackInfo<Value>& args) {
199+
WeakReference* weak_ref = Unwrap<WeakReference>(args.Holder());
200+
CHECK_GE(weak_ref->reference_count_, 1);
201+
weak_ref->reference_count_--;
202+
if (weak_ref->reference_count_ == 0) weak_ref->target_.SetWeak();
203+
}
204+
192205
SET_MEMORY_INFO_NAME(WeakReference)
193206
SET_SELF_SIZE(WeakReference)
194207
SET_NO_MEMORY_INFO()
195208

196209
private:
197210
Global<Object> target_;
211+
uint64_t reference_count_ = 0;
198212
};
199213

200214
static void GuessHandleType(const FunctionCallbackInfo<Value>& args) {
@@ -294,6 +308,8 @@ void Initialize(Local<Object> target,
294308
weak_ref->InstanceTemplate()->SetInternalFieldCount(1);
295309
weak_ref->SetClassName(weak_ref_string);
296310
env->SetProtoMethod(weak_ref, "get", WeakReference::Get);
311+
env->SetProtoMethod(weak_ref, "incRef", WeakReference::IncRef);
312+
env->SetProtoMethod(weak_ref, "decRef", WeakReference::DecRef);
297313
target->Set(context, weak_ref_string,
298314
weak_ref->GetFunction(context).ToLocalChecked()).Check();
299315

Collapse file

‎test/parallel/test-domain-error-types.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-domain-error-types.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --gc-interval=100 --stress-compaction
12
'use strict';
23

34
const common = require('../common');
@@ -6,6 +7,8 @@ const domain = require('domain');
67

78
// This test is similar to test-domain-multiple-errors, but uses a new domain
89
// for each errors.
10+
// The test flags are not essential, but serve as a way to verify that
11+
// https://github.com/nodejs/node/issues/28275 is fixed in debug mode.
912

1013
for (const something of [
1114
42, null, undefined, false, () => {}, 'string', Symbol('foo')

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.