Description
There is a race condition in SocketIO.EmitAsync (the overload with ack callback).
When two EmitAsync calls execute concurrently, both may end up sending the same packetId to the server, while the previous ID is silently orphaned in the ACK handler dictionary and its callback is never invoked.
As a result:
- An ACK handler can become permanently orphaned.
- The corresponding callback is never invoked.
- _ackHandlers may accumulate stale entries.
- Callers waiting for the ACK can hang indefinitely.
Root cause
The _packetId field is read after the lock is released, so another thread can increment it between the lock release and the SendAsync call:
lock (_ackHandlerLock)
{
_packetId++;
_ackHandlers.Add(_packetId, ack); // registers ID N
}
// ← another thread increments _packetId to N+1 here
await _session!.SendAsync(sessionData, _packetId, ...); // sends N+1 instead of N!
Steps to reproduce
Call EmitAsync with an ACK callback from multiple concurrent threads/tasks.
Evidence
The following log shows packet 247 was never sent (skipped), while 248 was sent
twice — both threads read _packetId = 248 after the race:
[WebSocket⬆] 42/.../sock,246 ← normal
Sending Text message...
Sent Text message.
[WebSocket⬇] 43/.../sock,246[{"hr":0}]
[WebSocket⬆] 42/.../sock,248 ← 247 missing! both threads sent 248
[WebSocket⬆] 42/.../sock,248 ← duplicate
Sending Text message...
Sending Text message...
Sent Text message.
Sent Text message.
[WebSocket⬇] 43/.../sock,248[{"hr":0}] ← server ACKed 248 twice
Deliver message to SocketIO, Type: Ack
[WebSocket⬇] 43/.../sock,248[{"hr":0}]
Deliver message to SocketIO, Type: Ack
[WebSocket⬆] 42/.../sock,249 ← resumes normally
The screenshot below shows _ackHandlers still holding key = 247 at runtime — the handler was registered but the corresponding message was never sent with that ID, so the ACK will never arrive and the callback leaks permanently.
Proposed fix:
Capture the packet ID inside the lock and use the captured value for both registration and transmission: SocketIO.EmitAsync line 279
int packetId;
lock (_ackHandlerLock)
{
_packetId++;
packetId = _packetId;
_ackHandlers.Add(packetId, ack);
}
await _session!.SendAsync(sessionData, packetId, cancellationToken);
I've reproduced the issue locally and verified that the above change resolves it.
Would the maintainers be open to a PR fixing this? I'd be happy to submit one.
Description
There is a race condition in
SocketIO.EmitAsync(the overload withackcallback).When two
EmitAsynccalls execute concurrently, both may end up sending the samepacketIdto the server, while the previous ID is silently orphaned in the ACK handler dictionary and its callback is never invoked.As a result:
Root cause
The
_packetIdfield is read after the lock is released, so another thread can increment it between the lock release and theSendAsynccall:Steps to reproduce
Call
EmitAsyncwith an ACK callback from multiple concurrent threads/tasks.Evidence
The following log shows packet
247was never sent (skipped), while248was senttwice — both threads read
_packetId = 248after the race:The screenshot below shows
_ackHandlersstill holdingkey = 247at runtime — the handler was registered but the corresponding message was never sent with that ID, so the ACK will never arrive and the callback leaks permanently.Proposed fix:
Capture the packet ID inside the lock and use the captured value for both registration and transmission:
SocketIO.EmitAsync line 279I've reproduced the issue locally and verified that the above change resolves it.
Would the maintainers be open to a PR fixing this? I'd be happy to submit one.