domain: avoid circular memory references

Avoid circular references that the JS engine cannot see through
because it involves an `async id` ⇒ `domain` link.

Using weak references is not a great solution, because it increases
the domain module’s dependency on internals and the added calls into
C++ may affect performance, but it seems like the least bad one.

PR-URL: https://github.com/nodejs/node/pull/25993
Fixes: https://github.com/nodejs/node/issues/23862
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Refael Ackermann <refack@gmail.com>

1 files changed, 10 insertions, 3 deletions

diff --git a/lib/domain.js b/lib/domain.js
index 8d96683312..031350746c 100644
--- a/lib/domain.js
+++ b/lib/domain.js
@@ -35,6 +35,10 @@ const {
 } = require('internal/errors').codes;
 const { createHook } = require('async_hooks');
 
+// TODO(addaleax): Use a non-internal solution for this.
+const kWeak = Symbol('kWeak');
+const { WeakReference } = internalBinding('util');
+
 // Overwrite process.domain with a getter/setter that will allow for more
 // effective optimizations
 var _domain = [null];
@@ -53,20 +57,22 @@ const asyncHook = createHook({
   init(asyncId, type, triggerAsyncId, resource) {
     if (process.domain !== null && process.domain !== undefined) {
       // If this operation is created while in a domain, let's mark it
-      pairing.set(asyncId, process.domain);
+      pairing.set(asyncId, process.domain[kWeak]);
       resource.domain = process.domain;
     }
   },
   before(asyncId) {
     const current = pairing.get(asyncId);
     if (current !== undefined) { // enter domain for this cb
-      current.enter();
+      // We will get the domain through current.get(), because the resource
+      // object's .domain property makes sure it is not garbage collected.
+      current.get().enter();
     }
   },
   after(asyncId) {
     const current = pairing.get(asyncId);
     if (current !== undefined) { // exit domain for this cb
-      current.exit();
+      current.get().exit();
     }
   },
   destroy(asyncId) {
@@ -167,6 +173,7 @@ class Domain extends EventEmitter {
     super();
 
     this.members = [];
+    this[kWeak] = new WeakReference(this);
     asyncHook.enable();
 
     this.on('removeListener', updateExceptionCapture);