Callbacks preventing garbage collection of objects

[achingbrain]

This is a follow-on to #215, I think the problem still exists, even after [email protected].

These references can keep threads alive:

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:63
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62           

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:67
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62           

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:71
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62           

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:75
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62           

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:79
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62           

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:85
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62           

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/node_modules/node-datachannel/polyfill/RTCPeerConnection.js:95
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:40           
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/test/peer.spec.js:62   

The referenced lines in RTCPeerConnection are all callbacks registered with the C++ object, for example line 63:

# ThreadSafeCallback callback
node:internal/async_hooks:202                                                                                          
file:///Users/alex/.../node_modules/node-datachannel/polyfill/RTCPeerConnection.js:63

https://github.com/murat-dogan/node-datachannel/blob/d866015dea085164fa110e34d4e5d86a8cbaa050/polyfill/RTCPeerConnection.js#L63

If I change the .close method of RTCPeerConnection the problem goes away:

close() {
  // close all channels before shutting down
  this.#dataChannels.forEach((channel) => {
    channel.close();
    });

    this.#peerConnection.close();
    this.#peerConnection.destroy();  // <-- add this line
}

I think this is because doDestroy calls doResetCallbacks which releases references to the JS callbacks.

To replicate, clone js-libp2p, apply this diff:

diff --git a/packages/transport-webrtc/package.json b/packages/transport-webrtc/package.json
index b38cbc5aa..01f03204e 100644
--- a/packages/transport-webrtc/package.json
+++ b/packages/transport-webrtc/package.json
@@ -39,8 +39,8 @@
   "scripts": {
     "generate": "protons src/private-to-private/pb/message.proto src/pb/message.proto",
     "build": "aegir build",
-    "test": "aegir test -t node -t browser -- --exit",
-    "test:node": "aegir test -t node --cov -- --exit",
+    "test": "aegir test -t node -t browser",
+    "test:node": "aegir test -t node --cov",
     "test:chrome": "aegir test -t browser --cov",
     "test:firefox": "aegir test -t browser -- --browser firefox",
     "lint": "aegir lint",
@@ -65,7 +65,7 @@
     "it-stream-types": "^2.0.1",
     "multiformats": "^13.1.0",
     "multihashes": "^4.0.3",
-    "node-datachannel": "^0.5.3",
+    "node-datachannel": "^0.5.4",
     "p-defer": "^4.0.0",
     "p-event": "^6.0.0",
     "p-timeout": "^6.1.2",
@@ -73,7 +73,8 @@
     "race-signal": "^1.0.2",
     "react-native-webrtc": "^118.0.1",
     "uint8arraylist": "^2.4.8",
-    "uint8arrays": "^5.0.2"
+    "uint8arrays": "^5.0.2",
+    "why-is-node-running": "^2.2.2"
   },
   "devDependencies": {
     "@chainsafe/libp2p-yamux": "^6.0.2",
diff --git a/packages/transport-webrtc/test/basics.spec.ts b/packages/transport-webrtc/test/basics.spec.ts
index c54488a7a..c649a9312 100644
--- a/packages/transport-webrtc/test/basics.spec.ts
+++ b/packages/transport-webrtc/test/basics.spec.ts
@@ -1,5 +1,8 @@
 /* eslint-disable @typescript-eslint/no-unused-expressions */
 
+// @ts-expect-error no types
+import log from 'why-is-node-running'
+
 import { noise } from '@chainsafe/libp2p-noise'
 import { yamux } from '@chainsafe/libp2p-yamux'
 import { circuitRelayTransport } from '@libp2p/circuit-relay-v2'
@@ -19,6 +22,10 @@ import pRetry from 'p-retry'
 import { webRTC } from '../src/index.js'
 import type { Libp2p, Connection, Stream, StreamHandler } from '@libp2p/interface'
 
+setTimeout(() => {
+  log()
+}, 5000).unref()
+
 async function createNode (): Promise<Libp2p> {
   return createLibp2p({
     addresses: {

Then install & build and run the node tests:

% cd js-libp2p
% npm i && npm run build
% cd packages/transport-webrtc
% npm run test:node -- --grep 'should connect'

You should see one test run, a brief pause then a list of all the handles keeping the process running:

% npm run test:node -- --grep 'should connect'

> @libp2p/[email protected] test:node
> aegir test -t node --cov --grep should connect

build

> @libp2p/[email protected] build
> aegir build

[10:37:34] tsc [started]
[10:37:35] tsc [completed]
[10:37:35] esbuild [started]
[10:37:35] esbuild [completed]
test node.js
Warning: Cannot find any files matching pattern "test/node.{js,cjs,mjs}"
Warning: Cannot find any files matching pattern "test/**/*.spec.{js,cjs,mjs}"
Warning: Cannot find any files matching pattern "dist/test/node.{js,cjs,mjs}"


  webrtc basic
    ✔ should connect (84ms)


  1 passing (92ms)

There are 43 handle(s) keeping the process running

# FILEHANDLE
node:internal/async_hooks:202

... lots of output here

Ignore the FILEHANDLE and KEYPAIRGENREQUEST entries - it's the ThreadSafeCallback callback entries causing the problem.