Serialize outbound frame writes to fix Stream.WriteAsync race (#211)#212
Merged
Conversation
- FixpClientSession: add a _writeLock (SemaphoreSlim) guarding every raw _stream.WriteAsync/FlushAsync call site (Negotiate, Establish, Terminate, Sequence, RetransmitRequest, application frames), so no two callers can ever write to the underlying Stream/SslStream concurrently. - EntryPointClient: add a _sendLock spanning reserve-seq -> encode -> write for every outbound application frame (Submit*/Replace*/Cancel*/ MassAction/SubmitCross/Quote*), so wire order always matches the order MsgSeqNum was assigned, closing the gap between the atomic NextOutboundSeqNum() reservation and the actual write. - Add regression tests: a deterministic FixpClientSession-level test proving writes no longer race/reorder, and an end-to-end test firing concurrent CancelAsync calls over a real TCP loopback peer asserting gap-free, ordered MsgSeqNum with no decode failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #211 — concurrent outbound sends could race on the shared
Stream.WriteAsync, letting frames land on the wire out ofMsgSeqNumorder or interleaved.Changes
FixpClientSession: added a_writeLock(SemaphoreSlim(1,1)) wrapping every raw_stream.WriteAsync/FlushAsynccall site — Negotiate, Establish, Terminate, Sequence, RetransmitRequest, and application frames — so no two callers ever write to the underlyingStream/SslStreamconcurrently.EntryPointClient: added a_sendLockspanning the full "reserve seq → encode → write" critical section for every outbound application frame (SubmitAsync,SubmitSimpleAsync,ReplaceAsync,ReplaceSimpleAsync,CancelAsync,MassActionAsync,SubmitCrossAsync,SendQuoteRequestAsync,SendQuoteAsync,CancelQuoteAsync), via a new privateSendOrderEntryFrameAsynchelper. This guarantees wire order always matches seq-assignment order, closing the gap between the already-atomicNextOutboundSeqNum()reservation and the actual write.Tests
ConcurrentSendOrderingTests.SendApplicationFrameAsync_SerializesConcurrentWrites_PreservesCallOrder— deterministic test using aDelayableStreamdouble that stalls a "slow" frame's write; verified it fails without the fix (fast frame wins the race and lands first) and passes with the fix.ConcurrentSendOrderingTests.ConcurrentCancelAsync_ArrivesInGapFreeSeqOrder— end-to-end test firing 8 concurrentCancelAsynccalls (mirroring the "Cancel All" trigger) over a real TCP loopbackInProcessFixpTestPeer, asserting gap-free, uniqueMsgSeqNumwith zero decode failures.Validation
dotnet build SbeB3EntryPointClient.slnx -c Release— 0 warnings/errorsdotnet test SbeB3EntryPointClient.slnx --no-build -c Release— all unit tests pass (213), conformance tests correctly skipped (no peer env vars)dotnet format SbeB3EntryPointClient.slnx --verify-no-changes --no-restore --severity warn— cleanCloses #211.