dmonad / lib0

Monorepo of isomorphic utility functions
MIT License
345 stars 62 forks source link

Jest reports BroadcastChannel leak #52

Closed erictheswift closed 1 year ago

erictheswift commented 1 year ago

Hi! πŸ‘‹

Firstly, thanks for your work on this project! πŸ™‚

Today I used patch-package to patch lib0@0.2.52 for the project I'm working on.

After I updated to node@18 and jest@28 I found out jest reports some of my tests leak.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  MESSAGEPORT
      at node_modules/lib0/broadcastchannel.js:66:16
      at Object.setIfUndefined (node_modules/lib0/map.js:49:24)
      at map.setIfUndefined (node_modules/lib0/broadcastchannel.js:64:3)
      at Object.subscribe (node_modules/lib0/broadcastchannel.js:83:39)
      at WebsocketProvider.bc.subscribe [as connectBc] (node_modules/y-websocket/src/y-websocket.js:345:7)
      at WebsocketProvider.connect (node_modules/y-websocket/src/y-websocket.js:394:12)
      at new WebsocketProvider (node_modules/y-websocket/src/y-websocket.js:305:12)

I found out that broadcastchannel module keeps BroadcastChannel alive for every room even with zero subscribers.

Here is the diff that solved my problem:

diff --git a/node_modules/lib0/dist/broadcastchannel-7655b66f.cjs b/node_modules/lib0/dist/broadcastchannel-7655b66f.cjs
index 12a6f51..fc82374 100644
--- a/node_modules/lib0/dist/broadcastchannel-7655b66f.cjs
+++ b/node_modules/lib0/dist/broadcastchannel-7655b66f.cjs
@@ -74,7 +74,17 @@ const subscribe = (room, f) => getChannel(room).subs.add(f);
  * @param {string} room
  * @param {function(any, any):any} f
  */
-const unsubscribe = (room, f) => getChannel(room).subs.delete(f);
+const unsubscribe = (room, f) => {
+    const channel = getChannel(room);
+    const unsubscribed = channel.subs.delete(f);
+    if (unsubscribed) {
+        if (channel.subs.size === 0) {
+            channel.bc.close();
+            channels.delete(room);
+        }
+    }
+    return unsubscribed;
+}

 /**
  * Publish data to all subscribers (including subscribers on this tab)

This issue body was partially generated by patch-package.

dmonad commented 1 year ago

Thanks for the PR @erictheswift. I'll release a patch in a bit

erictheswift commented 1 year ago

https://github.com/dmonad/lib0/actions/runs/3931623661/jobs/6723124025#step:6:105 2.0.59 patch build failed because of temporal test machine slowdown?

dmonad commented 1 year ago

I think node made a certain function faster. You don't have to worry about it.

dmonad commented 1 year ago

The function is still slower 90% of the time. Eventually, I'll move to the native encoding approach.