Ping pong memory leak fix for #511 (#512)

* New test for pingpong memory leak (failing).

* Shim once in relay ping mem test.

* Fix pong memory leak with .once.

Fixes #511.

* Fix missing global WebSocket on Node.

* Lint fix.

* Remove overkill WebSocket impl check.

abstract-relay.ts

@@ -193,12 +193,12 @@ export class AbstractRelay {
     return this.connectionPromise
   }
 
-  private async waitForPingPong() {
+  private waitForPingPong() {
-    return new Promise((res, err) => {
+    return new Promise(resolve => {
       // listen for pong
-      this.ws && this.ws.on ? this.ws.on('pong', () => res(true)) : err("ws can't listen for pong")
+      ;(this.ws as any).once('pong', () => resolve(true))
       // send a ping
-      this.ws && this.ws.ping && this.ws.ping()
+      this.ws!.ping!()
     })
   }
 
@@ -224,7 +224,7 @@ export class AbstractRelay {
       // wait for either a ping-pong reply or a timeout
       const result = await Promise.any([
         // browsers don't have ping so use a dummy req
-        this.ws && this.ws.ping && this.ws.on ? this.waitForPingPong() : this.waitForDummyReq(),
+        this.ws && this.ws.ping && (this.ws as any).once ? this.waitForPingPong() : this.waitForDummyReq(),
         new Promise(res => setTimeout(() => res(false), this.pingTimeout)),
       ])
       if (result) {
@@ -232,7 +232,7 @@ export class AbstractRelay {
         this.pingTimeoutHandle = setTimeout(() => this.pingpong(), this.pingFrequency)
       } else {
         // pingpong closing socket
-        if (this.ws?.readyState === WebSocket.OPEN) {
+        if (this.ws?.readyState === this._WebSocket.OPEN) {
           this.ws?.close()
         }
       }
@@ -434,7 +434,7 @@ export class AbstractRelay {
     this.closeAllSubscriptions('relay connection closed by us')
     this._connected = false
     this.onclose?.()
-    if (this.ws?.readyState === WebSocket.OPEN) {
+    if (this.ws?.readyState === this._WebSocket.OPEN) {
       this.ws?.close()
     }
   }

jsr.json

@@ -1,6 +1,6 @@
 {
   "name": "@nostr/tools",
-  "version": "2.17.0",
+  "version": "2.17.1",
   "exports": {
     ".": "./index.ts",
     "./core": "./core.ts",

package.json

@@ -1,7 +1,7 @@
 {
   "type": "module",
   "name": "nostr-tools",
-  "version": "2.17.0",
+  "version": "2.17.1",
   "description": "Tools for making a Nostr client.",
   "repository": {
     "type": "git",

relay.test.ts

@@ -129,9 +129,17 @@ test('ping-pong timeout (with native ping)', async () => {
       this.dispatchEvent(new Event('pong'))
     }
   }
-  ;(MockWebSocketClient.prototype as any).on = function (this: any, event: string, listener: () => void) {
+  ;(MockWebSocketClient.prototype as any).once = function (
+    this: any,
+    event: string,
+    listener: (...args: any[]) => void,
+  ) {
     if (event === 'pong') {
-      this.addEventListener('pong', listener)
+      const onceListener = (...args: any[]) => {
+        this.removeEventListener(event, onceListener)
+        listener.apply(this, args)
+      }
+      this.addEventListener('pong', onceListener)
     }
   }
 
@@ -166,7 +174,7 @@ test('ping-pong timeout (with native ping)', async () => {
     expect(closed).toBeTrue()
   } finally {
     delete (MockWebSocketClient.prototype as any).ping
-    delete (MockWebSocketClient.prototype as any).on
+    delete (MockWebSocketClient.prototype as any).once
   }
 })
 
@@ -217,6 +225,67 @@ test('ping-pong timeout (no-ping browser environment)', async () => {
   }
 })
 
+test('ping-pong listeners are cleaned up', async () => {
+  const mockRelay = new MockRelay()
+  let listenerCount = 0
+
+  // mock a native ping/pong mechanism
+  ;(MockWebSocketClient.prototype as any).ping = function (this: any) {
+    if (!mockRelay.unresponsive) {
+      this.dispatchEvent(new Event('pong'))
+    }
+  }
+
+  const originalAddEventListener = MockWebSocketClient.prototype.addEventListener
+  MockWebSocketClient.prototype.addEventListener = function (event, listener, options) {
+    if (event === 'pong') {
+      listenerCount++
+    }
+    // @ts-ignore
+    return originalAddEventListener.call(this, event, listener, options)
+  }
+
+  const originalRemoveEventListener = MockWebSocketClient.prototype.removeEventListener
+  MockWebSocketClient.prototype.removeEventListener = function (event, listener) {
+    if (event === 'pong') {
+      listenerCount--
+    }
+    // @ts-ignore
+    return originalRemoveEventListener.call(this, event, listener)
+  }
+
+  // the check in pingpong() is for .once() so we must mock it
+  ;(MockWebSocketClient.prototype as any).once = function (
+    this: any,
+    event: string,
+    listener: (...args: any[]) => void,
+  ) {
+    const onceListener = (...args: any[]) => {
+      this.removeEventListener(event, onceListener)
+      listener.apply(this, args)
+    }
+    this.addEventListener(event, onceListener)
+  }
+
+  try {
+    const relay = new Relay(mockRelay.url, { enablePing: true })
+    relay.pingTimeout = 50
+    relay.pingFrequency = 50
+
+    await relay.connect()
+    await new Promise(resolve => setTimeout(resolve, 175))
+
+    expect(listenerCount).toBeLessThan(2)
+
+    relay.close()
+  } finally {
+    delete (MockWebSocketClient.prototype as any).ping
+    delete (MockWebSocketClient.prototype as any).once
+    MockWebSocketClient.prototype.addEventListener = originalAddEventListener
+    MockWebSocketClient.prototype.removeEventListener = originalRemoveEventListener
+  }
+})
+
 test('reconnect on disconnect', async () => {
   const mockRelay = new MockRelay()
   const relay = new Relay(mockRelay.url, { enablePing: true, enableReconnect: true })