Skip to content

Commit da04267

Browse files
seungeonchoidarrachequesne
authored andcommitted
fix: cleanup pending acks on timeout to prevent memory leak (#5442)
When using `emitWithAck` with a timeout, if clients didn't respond and the timeout triggers, the ack callbacks remained in `socket.acks` Map indefinitely, causing a memory leak. Related: #4984
1 parent 74599a6 commit da04267

4 files changed

Lines changed: 21 additions & 2 deletions

File tree

packages/socket.io-adapter/lib/cluster-adapter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ export abstract class ClusterAdapter extends Adapter {
508508
}, opts.flags!.timeout);
509509
}
510510

511-
super.broadcastWithAck(packet, opts, clientCountCallback, ack);
511+
return super.broadcastWithAck(packet, opts, clientCountCallback, ack);
512512
}
513513

514514
override async addSockets(opts: BroadcastOptions, rooms: Room[]) {

packages/socket.io-adapter/lib/in-memory-adapter.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,14 @@ export class Adapter extends EventEmitter {
229229
});
230230

231231
clientCountCallback(clientCount);
232+
233+
return {
234+
cleanup: () => {
235+
this.apply(opts, (socket) => {
236+
socket.acks.delete(packet.id);
237+
});
238+
},
239+
};
232240
}
233241

234242
private _encode(packet: unknown, packetOpts: Record<string, unknown>) {

packages/socket.io/lib/broadcast-operator.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,9 +232,11 @@ export class BroadcastOperator<EmitEvents extends EventsMap, SocketData>
232232
const ack = data.pop() as (...args: any[]) => void;
233233
let timedOut = false;
234234
let responses: any[] = [];
235+
let cleanupPendingAcks: (() => void) | undefined;
235236

236237
const timer = setTimeout(() => {
237238
timedOut = true;
239+
cleanupPendingAcks?.();
238240
ack.apply(this, [
239241
new Error("operation has timed out"),
240242
this.flags.expectSingleResponse ? null : responses,
@@ -259,7 +261,7 @@ export class BroadcastOperator<EmitEvents extends EventsMap, SocketData>
259261
}
260262
};
261263

262-
this.adapter.broadcastWithAck(
264+
const result = this.adapter.broadcastWithAck(
263265
packet,
264266
{
265267
rooms: this.rooms,
@@ -279,6 +281,10 @@ export class BroadcastOperator<EmitEvents extends EventsMap, SocketData>
279281
},
280282
);
281283

284+
if (result && typeof result.cleanup === "function") {
285+
cleanupPendingAcks = result.cleanup;
286+
}
287+
282288
this.adapter.serverCount().then((serverCount) => {
283289
expectedServerCount = serverCount;
284290
checkCompleteness();

packages/socket.io/test/messaging-many.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,11 @@ describe("messaging many", () => {
534534
// @ts-ignore
535535
expect(err.responses).to.contain(1, 2);
536536

537+
for (const [, serverSocket] of io.of("/").sockets) {
538+
// @ts-ignore accessing private acks map to verify cleanup
539+
expect(serverSocket.acks.size).to.be(0);
540+
}
541+
537542
success(done, io, socket1, socket2, socket3);
538543
}
539544
});

0 commit comments

Comments
 (0)