Fix #4984: Clean up socket.acks on emitWithAck timeout to prevent memory leak

[cobyfrombrooklyn-bot]

Problem

When using emitWithAck with a timeout, if clients don't respond before the timeout fires, the ack callbacks stored in socket.acks are never cleaned up. Each timed-out broadcast leaves orphaned entries in the acks Map, causing unbounded memory growth.

As reported in #4984, with 500 clients and frequent broadcasts where clients don't acknowledge, memory usage grew to 1.5GB in 3 hours.

Root Cause

broadcastWithAck in the adapter stores an ack callback in socket.acks.set(packet.id, ack) for each matching socket. When a client responds, socket.onack() deletes the entry. But when the timeout fires, only the caller's ack callback is invoked with an error; the per-socket entries in socket.acks are never removed.

Fix

1. Added removeAcks(packetId, opts) to the in-memory adapter. It iterates the same set of matching sockets (using apply()) and deletes the ack entry for the given packet ID.
2. Modified the timeout handler in BroadcastOperator.emit() to call removeAcks() before invoking the error callback. Uses a feature check (typeof this.adapter.removeAcks === 'function') for backward compatibility with custom adapters.

Test

Added should clean up socket.acks after emitWithAck timeout (memory leak fix) in test/messaging-many.ts. The test:

• Connects two clients; one responds, one intentionally doesn't (simulating timeout)
• Calls emitWithAck with a 200ms timeout
• After the timeout error, verifies that all server-side socket acks maps are empty

The test fails without the fix (acks map retains the orphaned entry) and passes with it.

Tested locally on macOS ARM (Apple Silicon). All existing ack-related tests continue to pass.

[idirdev]

Solid fix. The root cause analysis is accurate — socket.acks.set(packet.id, ack) in broadcastWithAck creates entries that are only cleaned up when a client responds via socket.onack(). On timeout, the error callback fires but the per-socket entries stay forever.

A few notes after reading the diff:

The approach is sound. Using this.apply(opts, ...) in removeAcks reuses the adapter's existing socket-matching logic, which keeps the cleanup consistent with how the broadcast was originally scoped. The typeof check for backward compatibility with custom adapters is a good touch.

Minor edge case: Between the broadcast and the timeout firing, room membership could theoretically change (a socket leaves a room or disconnects). This means removeAcks might not iterate the exact same set of sockets that received the original broadcast. In practice this is negligible — a disconnected socket would be garbage collected anyway, and a socket that left a room but still has an orphaned ack entry is a tiny leak compared to the unbounded growth without this fix.

broadcastOpts extraction is a clean refactoring — avoids duplicating the rooms/except/flags object and ensures the same filter is used for both broadcast and cleanup.

Test coverage looks good. The two-client pattern (one responds, one times out) directly validates the fix. One potential addition: a test verifying that when all clients respond before the timeout, no cleanup call is needed (i.e., socket.acks.size === 0 even without removeAcks). This would document the existing happy-path behavior alongside the fix.