From 1cd4fdb75d35c729f2060b64d6c096ea57ed9bf3 Mon Sep 17 00:00:00 2001 From: evanpelle Date: Sat, 26 Jul 2025 13:51:21 -0700 Subject: [PATCH] fix ws mem leak (again) (#1591) ## Description: The initial memory leak where websockets had references to GameServers has been fixed. But a new memory leak has been introduced. A memory dump showed that WebSockerServer was hanging onto 10s of thousands of closed websockets. I believe it's because removeAllListeners was removing internal clean up listeners, from the comment: * It is bad practice to remove listeners added elsewhere in the code, * particularly when the EventEmitter instance was created by some other * component or module (e.g. sockets or file streams). This PR ensures that removeAllListeners is only called in the close listener, which is executed after internal cleanup. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced - [x] I have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: evan --- src/server/GameServer.ts | 8 +------- src/server/Worker.ts | 10 ---------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/server/GameServer.ts b/src/server/GameServer.ts index 73d44195c..42524faee 100644 --- a/src/server/GameServer.ts +++ b/src/server/GameServer.ts @@ -176,7 +176,6 @@ export class GameServer { client.isDisconnected = existing.isDisconnected; client.lastPing = existing.lastPing; - existing.ws.removeAllListeners(); this.activeClients = this.activeClients.filter((c) => c !== existing); } @@ -186,7 +185,7 @@ export class GameServer { this.allClients.set(client.clientID, client); - client.ws.removeAllListeners(); + client.ws.removeAllListeners("message"); client.ws.on( "message", gatekeeper.wsHandler(client.ip, async (message: string) => { @@ -205,7 +204,6 @@ export class GameServer { } satisfies ServerErrorMessage), ); client.ws.close(1002, "ClientMessageSchema"); - client.ws.removeAllListeners(); return; } const clientMsg = parsed.data; @@ -263,7 +261,6 @@ export class GameServer { }); client.ws.on("error", (error: Error) => { if ((error as any).code === "WS_ERR_UNEXPECTED_RSV_1") { - client.ws.removeAllListeners(); client.ws.close(1002, "WS_ERR_UNEXPECTED_RSV_1"); } }); @@ -405,7 +402,6 @@ export class GameServer { // Close all WebSocket connections clearInterval(this.endTurnIntervalID); this.websockets.forEach((ws) => { - ws.removeAllListeners(); if (ws.readyState === WebSocket.OPEN) { ws.close(1000, "game has ended"); } @@ -463,7 +459,6 @@ export class GameServer { }); if (client.ws.readyState === WebSocket.OPEN) { client.ws.close(1000, "no heartbeats received, closing connection"); - client.ws.removeAllListeners(); } } else { alive.push(client); @@ -558,7 +553,6 @@ export class GameServer { this.activeClients = this.activeClients.filter( (c) => c.clientID !== clientID, ); - client.ws.removeAllListeners(); this.kickedClients.add(clientID); } else { this.log.warn(`cannot kick client, not found in game`, { diff --git a/src/server/Worker.ts b/src/server/Worker.ts index 96efde652..bad56d0a5 100644 --- a/src/server/Worker.ts +++ b/src/server/Worker.ts @@ -316,7 +316,6 @@ export function startWorker() { error: error.toString(), } satisfies ServerErrorMessage), ); - ws.removeAllListeners(); ws.close(1002, "ClientJoinMessageSchema"); return; } @@ -334,7 +333,6 @@ export function startWorker() { error, } satisfies ServerErrorMessage), ); - ws.removeAllListeners(); ws.close(1002, "ClientJoinMessageSchema"); return; } @@ -352,7 +350,6 @@ export function startWorker() { const result = await verifyClientToken(clientMsg.token, config); if (result === false) { log.warn("Unauthorized: Invalid token"); - ws.removeAllListeners(); ws.close(1002, "Unauthorized"); return; } @@ -365,7 +362,6 @@ export function startWorker() { if (claims === null) { if (allowedFlares !== undefined) { log.warn("Unauthorized: Anonymous user attempted to join game"); - ws.removeAllListeners(); ws.close(1002, "Unauthorized"); return; } @@ -374,7 +370,6 @@ export function startWorker() { const result = await getUserMe(clientMsg.token, config); if (result === false) { log.warn("Unauthorized: Invalid session"); - ws.removeAllListeners(); ws.close(1002, "Unauthorized"); return; } @@ -389,7 +384,6 @@ export function startWorker() { log.warn( "Forbidden: player without an allowed flare attempted to join game", ); - ws.removeAllListeners(); ws.close(1002, "Forbidden"); return; } @@ -406,7 +400,6 @@ export function startWorker() { ); if (allowed !== true) { log.warn(`Custom flag ${allowed}: ${clientMsg.flag}`); - ws.removeAllListeners(); ws.close(1002, `Custom flag ${allowed}`); return; } @@ -422,7 +415,6 @@ export function startWorker() { ); if (allowed !== true) { log.warn(`Pattern ${allowed}: ${clientMsg.pattern}`); - ws.removeAllListeners(); ws.close(1002, `Pattern ${allowed}`); return; } @@ -457,7 +449,6 @@ export function startWorker() { // Handle other message types } catch (error) { - ws.removeAllListeners(); ws.close(1011, "Internal server error"); log.warn( `error handling websocket message for ${ipAnonymize(ip)}: ${error}`.substring( @@ -470,7 +461,6 @@ export function startWorker() { ); ws.on("error", (error: Error) => { - ws.removeAllListeners(); if ((error as any).code === "WS_ERR_UNEXPECTED_RSV_1") { ws.close(1002, "WS_ERR_UNEXPECTED_RSV_1"); }