Memory leak - polyfill

[lortonx]

I found a memory leak in the polyfill, the context data is stored in memory. I started removing #peerConnection from _RTCPeerConnection (like _RTCPeerConnection._peerConnection = null ), the situation becomes to normal. I'm not good at fixing leaks properly. I hope this information will help the developer fix the memory leak

[lortonx]

Functions passed as arguments to NodeDataChannel.PeerConnection cannot be released by the GC

onStateChange
onIceStateChange
onSignalingStateChange
onGatheringStateChange
onDataChannel
onLocalDescription
onLocalCandidate

[murat-dogan]

Hello and thank you for reporting.
I will check it soon and update the issue.

[lortonx]

This snippet probably fixes the leak, at least I don't notice any leaks anymore with this one

import NodeDataChannel from 'node-datachannel';
function dummy() {}
class _PeerConnection extends NodeDataChannel.PeerConnection {
    hooked = {
        onStateChange: null,
        onIceStateChange: null,
        onSignalingStateChange: null,
        onGatheringStateChange: null,
        onDataChannel: null,
        onLocalDescription: null,
        onLocalCandidate: null
    };
    constructor(a, b) {
        super(a, b);
    }
    onStateChange(l) {
        this.hooked.onStateChange = l;
        super.onStateChange((param) => this.hooked.onStateChange(param));
    }
    onIceStateChange(l) {
        this.hooked.onIceStateChange = l;
        super.onIceStateChange((param) => this.hooked.onIceStateChange(param));
    }
    onSignalingStateChange(l) {
        this.hooked.onSignalingStateChange = l;
        super.onSignalingStateChange((param) => this.hooked.onSignalingStateChange(param));
    }
    onGatheringStateChange(l) {
        this.hooked.onGatheringStateChange = l;
        super.onGatheringStateChange((param) => this.hooked.onGatheringStateChange(param));
    }
    onDataChannel(l) {
        this.hooked.onDataChannel = l;
        super.onDataChannel((param) => this.hooked.onDataChannel(param));
    }
    onLocalDescription(l) {
        this.hooked.onLocalDescription = l;
        super.onLocalDescription((param1, param2) => this.hooked.onLocalDescription(param1, param2));
    }
    onLocalCandidate(l) {
        this.hooked.onLocalCandidate = l;
        super.onLocalCandidate((param1, param2) => this.hooked.onLocalCandidate(param1, param2));
    }
    removeEvents() {
        for (let key in this.hooked) {
            this.hooked[key] = function () {};
        }
        setTimeout(() => (this.hooked = null), 30);

        super.onStateChange(dummy);
        super.onIceStateChange(dummy);
        super.onSignalingStateChange(dummy);
        super.onGatheringStateChange(dummy);
        super.onDataChannel(dummy);
        super.onLocalDescription(dummy);
        super.onLocalCandidate(dummy);
        this.hooked = null;
    }
    close() {
        this.removeEvents();
        super.close();
    }
}
NodeDataChannel.PeerConnection = _PeerConnection;

[murat-dogan]

Hello,
I don't see any problem actually.

In your snippet, you are resetting all functions and then closing the connection.
I guess you could do 2 things for GC;

• delete PeerConnection object. This should release everything.
• reset all callbacks without deleting the pc, this should release other objects

[achingbrain]

I think I've come across a similar problem when trying to remove this forced exit from a test suite.

Something is keeping the Node.js process from exiting - running with why-is-node-running reveals:

# 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/src/private-to-private/transport.js:109
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/transport-webrtc/dist/src/private-to-private/transport.js:47 
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/libp2p/dist/src/upgrader.js:468                              
file:///Users/alex/Documents/Workspaces/libp2p/js-libp2p/packages/libp2p/dist/src/upgrader.js:302                              
node:internal/process/task_queues:95  

Line 67 of the RTCPeerConnection polyfill is where the onIceStateChange callback is set up which is wrapped in a ThreadSafeCallback.

nulling this.#peerConnection doesn't seem to help, nor does resetting callbacks.

Is there some extra teardown that's needed when the peer connection is closed?

[paullouisageneau]

@achingbrain It appears the ICE state change callback is not reset on destruction in the wrapper. I'll push a fix.

[paullouisageneau]

@murat-dogan It should be fixed by #221

[murat-dogan]

Good catch!
Thanks @paullouisageneau

I merged it.