PeerConnection.close() Method is Blocking the Event Loop

[mertushka]

Describe the Bug

In certain cases, the PeerConnection.close() polyfill seems to block the event loop. Profiling has shown that the PeerConnection.close() method sometimes takes ~1 second to execute instead of the usual few milliseconds.

Additionally, the datachannel.readyState does not always correctly reflect its real state. This inconsistency can lead to issues such as attempting to send messages on a data channel that appears open but is not actually open. Check the additional context for more insight.

To Reproduce

Steps to reproduce the issue:

1. Open a WebRTC connection.
2. Trigger a PeerConnection.close() call.
3. Observe that the event loop may be blocked, even for ~1 second.
4. Check the datachannel.readyState before and after the .close() call to confirm state inconsistencies.

Expected Behavior

• The PeerConnection.close() polyfill should clean up the connection asynchronously and avoid blocking the event loop.
• The datachannel.readyState should accurately reflect the real state of the data channel.

Device Information

• Node.js Version: 20.x LTS

Additional Context

See: mertushka/haxball.js#44

[mertushka]

node-datachannel/src/polyfill/RTCDataChannel.ts

Lines 192 to 195 in dd3954e

	close(): void {
	this.#closeRequested = true;
	this.#dataChannel.close();
	}

Fix

I was able to fix the issue by updating the close() method in the RTCDataChannel polyfill to use setImmediate. Specifically, I wrapped the this.#dataChannel.close() call in a setImmediate function like this:

close(): void {
    setImmediate(() => {
        this.#closeRequested = true;
        this.#dataChannel.close();
    });
}

This change ensures that the close() operation is executed asynchronously, preventing it from blocking the event loop. After applying this fix, the blocking caused by PeerConnection.close() are no longer occurring.

@murat-dogan Let me know if this is an ideal solution so i can make a PR.

[murat-dogan]

Hello @mertushka and thank you for sharing the solution.
The solution seems reasonable.

The only point is, can we keep that part out of setImmediate;
this.#closeRequested = true;

What do you think?

[mertushka]

@murat-dogan Done with the #327 PR.

[mertushka]

@murat-dogan Can you release the new version about this? This is considered as a major bug and i must have the fix on my module ASAP. Thanks in advance.

[murat-dogan]

https://github.com/murat-dogan/node-datachannel/releases/tag/v0.25.0

Has been released.

[mertushka]

@murat-dogan This issue should be reopened as <DataChannel>.readyState still does not accurately reflect its actual state.

Relevant issue: mertushka/haxball.js#49

[mertushka]

Hey @paullouisageneau, was this issue related to paullouisageneau/libdatachannel#1439? If so, we can drop the setImmediate() wrap i think.