fix(transit/delta-message): ReceiveAllAsync survives transient transport disconnects via _receiveEof#331
Merged
Conversation
…ort disconnects via _receiveEof (fixes #297)
Owner
Author
|
CI is failing on Cannot approve while CI is red. Suggest re-running the |
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
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.
Fixes #297.
Problem
DeltaMessageTransit<T>.ReceiveAllAsyncusedIsConnectedas a termination condition:IsConnectedflips tofalseduring every transient transport disconnect — TCP RST, WebSocket reconnect, keepalive miss, etc. — even when auto-reconnect is configured to recover within milliseconds. The enumerable terminated mid-session with no exception, indistinguishable from a clean peer close. The fallbackelse if (!IsConnected) break;after a null return additionally conflated mid-stream control frames (resync requests, type0x02; delta-before-baseline triggering an outbound resync) with real end-of-stream.MessageTransit.ReceiveAllAsyncsolved this in #177 by using a_receiveEofflag set only on real EOF. The same pattern was not applied toDeltaMessageTransit.Fix
private volatile bool _receiveEof;field.ReadMessageAsyncsets_receiveEof = truewhen the underlying read returns 0 bytes (real EOF) — both in the length-prefix loop and the payload loop.ReceiveAllAsyncto mirrorMessageTransit.ReceiveAllAsync:!cancellationToken.IsCancellationRequested && !_disposed(noIsConnected).OperationCanceledExceptionandObjectDisposedExceptionfromReceiveAsyncterminate viayield break._receiveEofis the sole real-EOF terminator.0x02, or delta-before-baseline sending an outbound resync request) and the loop continues to wait for the next data frame.Regression test
DeltaMessageTransitReceiveAllReconnectTests:ReceiveAllAsync_TransportDisconnectMidStream_ContinuesAfterReconnect— uses aScriptedReadChanneltest double whoseIsConnectedtoggles and whoseReadAsyncparks until frames are enqueued. The consumer's foreach body is gated on aTaskCompletionSourceso the iterator'swhile-check fires afterIsConnectedflips to false (deterministic — no race timing). Pre-fix: enumerable terminates after 1 frame. Post-fix: enumerable continues, second frame is delivered.ReceiveAllAsync_ReadReturnsZero_TerminatesViaEof— confirms real EOF still terminates the enumerable (via_receiveEof).Verification
ReceiveAllAsync_TransportDisconnectMidStream_ContinuesAfterReconnectFAILS withExpected: 2, Actual: 1— proving the bug.dotnet test tests/NetConduit.Transit.DeltaMessage.UnitTests→ 80/80 pass.Scope
Stays within NetConduit's stated scope (stream multiplexer). The fix only affects the in-process iteration contract of an existing optional transit; no new auth, encryption, routing, persistence, or hostile-peer concerns are introduced.