Frames that arrive before StartMessageProcessor has run bypass the bounded queue, and the obvious fix is blocked by an unrelated coupling.
The window
OnceOpen ends like this:
try
{
connectionManager.ResolveAllAwaiting();
if (OnConnected is not null)
{
await OnConnected?.Invoke(); // consumer subscribes here
}
...
}
...
StartPingTimer();
StartMessageProcessor(); // channel created only now
Subscribing from an OnConnected handler is the normal pattern — it is what the wallet consuming this SDK does. The node can answer that subscription before the handler returns, so the first frames find _streamMessageChannel still null and take the fallback in EnqueueStreamMessage:
else
{
_ = ProcessStreamMessageFireAndForgetAsync(frame);
}
Those frames are outside everything the queue provides: StreamMessageQueueCapacity does not apply to them, DroppedStreamMessages does not count them, and they are dispatched concurrently rather than one at a time — so the first events after connecting are exactly the ones that can arrive out of order.
Why the one-line fix does not work
Moving StartMessageProcessor() ahead of the try looks right and fails, measurably: TestUDroppedStreamMessagesCountsWhatTheConsumerNeverSaw goes from 10 evictions to 0.
The reason is a coupling that has nothing to do with message processing:
private void StartPingTimer()
{
...
StopPingTimerSync(); // and this calls StopMessageProcessor()
So starting the processor before StartPingTimer has it torn down again moments later, and the fallback path handles every frame from then on. Stopping a ping timer should not stop the message queue; that is the actual defect behind this one.
What to do
- Remove
StopMessageProcessor() from StopPingTimerSync, so the two lifecycles are independent.
- Add explicit
StopMessageProcessor() calls where stopping is genuinely intended — Disconnect, DisconnectAndWaitAsync, OnceClose, OnConnectHandlerFailedAsync, and the ChangeServer / RetireCurrentSessionAndReconnectAsync paths, which currently rely on the side effect.
- Then move
StartMessageProcessor() ahead of the OnConnected callback, so the channel exists before consumer code can subscribe.
- Once frames can no longer arrive without a channel, the fire-and-forget fallback in
EnqueueStreamMessage covers only the not-connected case — worth checking whether it can go entirely.
A test can pin this: subscribe from an OnConnected handler, feed frames from inside that handler, and assert DroppedStreamMessages accounts for them under a small StreamMessageQueueCapacity — impossible today, since they never reach the queue.
Found by review of #111. Not introduced there: the ordering predates it, and the browser bypass removed in that PR was a separate path.
Frames that arrive before
StartMessageProcessorhas run bypass the bounded queue, and the obvious fix is blocked by an unrelated coupling.The window
OnceOpenends like this:Subscribing from an
OnConnectedhandler is the normal pattern — it is what the wallet consuming this SDK does. The node can answer that subscription before the handler returns, so the first frames find_streamMessageChannelstill null and take the fallback inEnqueueStreamMessage:Those frames are outside everything the queue provides:
StreamMessageQueueCapacitydoes not apply to them,DroppedStreamMessagesdoes not count them, and they are dispatched concurrently rather than one at a time — so the first events after connecting are exactly the ones that can arrive out of order.Why the one-line fix does not work
Moving
StartMessageProcessor()ahead of thetrylooks right and fails, measurably:TestUDroppedStreamMessagesCountsWhatTheConsumerNeverSawgoes from 10 evictions to 0.The reason is a coupling that has nothing to do with message processing:
So starting the processor before
StartPingTimerhas it torn down again moments later, and the fallback path handles every frame from then on. Stopping a ping timer should not stop the message queue; that is the actual defect behind this one.What to do
StopMessageProcessor()fromStopPingTimerSync, so the two lifecycles are independent.StopMessageProcessor()calls where stopping is genuinely intended —Disconnect,DisconnectAndWaitAsync,OnceClose,OnConnectHandlerFailedAsync, and theChangeServer/RetireCurrentSessionAndReconnectAsyncpaths, which currently rely on the side effect.StartMessageProcessor()ahead of theOnConnectedcallback, so the channel exists before consumer code can subscribe.EnqueueStreamMessagecovers only the not-connected case — worth checking whether it can go entirely.A test can pin this: subscribe from an
OnConnectedhandler, feed frames from inside that handler, and assertDroppedStreamMessagesaccounts for them under a smallStreamMessageQueueCapacity— impossible today, since they never reach the queue.Found by review of #111. Not introduced there: the ordering predates it, and the browser bypass removed in that PR was a separate path.