suhaildawood / SvelteKit-integrated-WebSocket

First-class support for WebSockets within SvelteKit by attaching a WebSocket server to the global state.
MIT License
219 stars 13 forks source link

Possible EventEmitter memory leak #2

Closed lnfel closed 1 year ago

lnfel commented 1 year ago

I've followed the implementation details but it seems startupWebsocketServer is fired for every page navigation, because it is included inside sveltekit hooks handle which is triggered for each page navigation:

Here is the error, this occurs after several page navigation (in my case it occured on 6th - 9th) once we get passed through limit and node sends us a warning:

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 connection listeners added to [WebSocketServer]. Use emitter.setMaxListeners() to increase limit

For some reaseon setting wssInitialized to true after startup does not prevent re-running of the function resulting to many connection instances.

Right now I've move the startup code outside of sveltekit handle hooks sequence. The drawback is we can no longer access ws via locals.

suhaildawood commented 1 year ago

Hi @lnfel , thanks for bringing this up! I played around with a multi-page example but was unable to reproduce this. Each time the hook handle function was triggered but startupWebsocketServer short-circuits after the setup function runs for the first time. In the screenshot below you can see [wss:kit] setup only logs once:

image

Could you provide some mode details on how you ran into the MaxListenersExceededWarning?

lnfel commented 1 year ago

I figured it out just now, it is happening because I had wssInitialized = true inside wss.on 'connection' like this:

// hooks.server.js

let wssInitialized = false
const startupWebsocketServer = () => {
  if (wssInitialized) return;
  console.log('[wss:kit] startupWebsocketServer')

  /**
   * @type {import('ws').Server<import('ws').WebSocket>}
   */
  const wss = globalThis[GlobalThisWSS]

  if (wss !== undefined) {
    wss.on('connection', (ws, request) => {
      // omitted code...

      // this is the issue
      // wssInitialized = true
    })

    // this is the correct placement
    wssInitialized = true
  }
}

Everything works fine now.