From 152d903c6c1523787b409c599c4678c87e80556b Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 6 Jul 2026 18:47:39 +0000 Subject: [PATCH 1/2] Fix #208: skip stale-snapshot resume on SessionVerId bump HydrateFromSnapshotAsync only compared snapshot.SessionId against the current session, never SessionVerId. After a cold-start EstablishReuse reject forces a fresh Negotiate (bumping SessionVerId), the outbound app seq counter and inbound/outstanding-order state were still resumed from the pre-bump snapshot, silently desyncing MsgSeqNum from what the venue expects for the new session. Now HydrateFromSnapshotAsync also checks snapshot.SessionVerId against the (possibly bumped) options.SessionVerId and skips resuming state when they differ, logging via new StaleSnapshotSessionVerIdIgnored. Adds a regression test verifying the outbound seq starts at 1 (not the stale snapshot's LastOutboundSeqNum + 1) after a rejected cold-start reattach, via new PeekNextOutboundSeqNumForTesting test hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/B3.EntryPoint.Client/EntryPointClient.cs | 12 +++++++ .../Logging/LogMessages.cs | 4 +++ .../ColdStartResumeTests.cs | 32 +++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/src/B3.EntryPoint.Client/EntryPointClient.cs b/src/B3.EntryPoint.Client/EntryPointClient.cs index c845bfc..6b19bd6 100644 --- a/src/B3.EntryPoint.Client/EntryPointClient.cs +++ b/src/B3.EntryPoint.Client/EntryPointClient.cs @@ -513,6 +513,17 @@ private async ValueTask HydrateFromSnapshotAsync(CancellationToken ct) _options.Logger.StaleSnapshotIgnored(snapshot.SessionId, _options.SessionId); return; } + if (snapshot.SessionVerId != _options.SessionVerId) + { + // A SessionVerId bump (e.g. cold-start EstablishReuse rejected, + // falling back to a fresh Negotiate; see #208) means this is a + // logically new negotiated session: FIXP resets application-level + // sequence numbering to 1 on both sides regardless of the old + // snapshot's contents, so resuming from it here would silently + // desync the outbound MsgSeqNum from what the venue expects. + _options.Logger.StaleSnapshotSessionVerIdIgnored(snapshot.SessionVerId, _options.SessionVerId, _options.SessionId); + return; + } _session!.ResumeOutboundSeqNum(snapshot.LastOutboundSeqNum + 1UL); lock (_inboundGapGate) { @@ -856,6 +867,7 @@ internal Task StopActiveSessionForTestingAsync(CancellationToken ct = default) internal void HandleInboundEventForTesting(EntryPointEvent evt) => OnInboundEventForPersistence(evt); internal void BindRetransmitForTesting(RetransmitRequestHandler handler) => _retransmit = handler; internal bool HasActiveSessionForTesting => _session is not null || _tcp is not null; + internal ulong PeekNextOutboundSeqNumForTesting() => _session?.PeekNextOutboundSeqNum() ?? 0UL; internal (ulong contiguous, ulong highest, int pending, bool gapInFlight) GetInboundGapStateForTesting() { lock (_inboundGapGate) diff --git a/src/B3.EntryPoint.Client/Logging/LogMessages.cs b/src/B3.EntryPoint.Client/Logging/LogMessages.cs index 7800568..6599aa8 100644 --- a/src/B3.EntryPoint.Client/Logging/LogMessages.cs +++ b/src/B3.EntryPoint.Client/Logging/LogMessages.cs @@ -93,6 +93,10 @@ internal static partial class LogMessages Message = "Cold-start resume rejected (code={Code}); falling back to Negotiate from SessionVerID={SessionVerId}")] public static partial void ColdResumeFallbackToNegotiate(this ILogger logger, B3.Entrypoint.Fixp.Sbe.V6.EstablishRejectCode code, uint sessionVerId); + [LoggerMessage(EventId = 3009, Level = LogLevel.Information, + Message = "Persisted snapshot SessionVerID={Persisted} != current {Current} for SessionID={SessionId}; new negotiated session starts fresh, ignoring stale sequence/order state.")] + public static partial void StaleSnapshotSessionVerIdIgnored(this ILogger logger, uint persisted, uint current, uint sessionId); + // ---------------- Warning (4000–4999) ---------------- [LoggerMessage(EventId = 4000, Level = LogLevel.Warning, diff --git a/tests/B3.EntryPoint.Client.Tests/ColdStartResumeTests.cs b/tests/B3.EntryPoint.Client.Tests/ColdStartResumeTests.cs index 837021b..7195d84 100644 --- a/tests/B3.EntryPoint.Client.Tests/ColdStartResumeTests.cs +++ b/tests/B3.EntryPoint.Client.Tests/ColdStartResumeTests.cs @@ -97,6 +97,38 @@ public async Task ColdResume_EstablishRejectedUnnegotiated_FallsBackToNegotiate_ Assert.Equal(107u, options.SessionVerId); } + [Fact] + public async Task ColdResume_EstablishRejectedUnnegotiated_DoesNotResumeOutboundSeqFromStaleSnapshot() + { + // Regression for #208: on a recoverable EstablishReuse reject, the + // client bumps SessionVerId and falls back to a fresh Negotiate. + // HydrateFromSnapshotAsync must NOT resume the outbound app seq + // counter from the pre-bump snapshot (a new SessionVerId means the + // wire-level sequence numbering starts fresh regardless of the old + // snapshot's contents). + await using var peer = new InProcessFixpTestPeer(new TestPeerOptions + { + RejectEstablishWithoutPriorNegotiate = true, + }); + peer.Start(); + + var options = Options(peer); + options.ConnectMode = ConnectMode.EstablishReuseThenNegotiate; + // Stale snapshot from the prior (rejected) SessionVerId, with a + // large outbound counter that must NOT be carried forward. + options.SessionStateStore = new SnapshotStore(Snapshot(sessionVerId: 7u, lastOut: 22u)); + options.NextSessionVerIdSelector = prev => prev + 1u; + await using var client = new EntryPointClient(options); + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + + await client.ConnectAsync(cts.Token); + + Assert.Equal(8u, options.SessionVerId); + // Fresh session (new SessionVerId): outbound app seq must start at 1, + // not resume at 23 (stale snapshot's LastOutboundSeqNum + 1). + Assert.Equal(1u, client.PeekNextOutboundSeqNumForTesting()); + } + [Fact] public async Task ColdResume_NoSnapshot_PlainNegotiate_NoVerIdChange() { From a3bba6dc2f305c563739b1b0a6006b8bb971d424 Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 6 Jul 2026 18:53:12 +0000 Subject: [PATCH 2/2] Scope #208 fix to cold-resume reject fallback only The prior fix compared snapshot.SessionVerId against the current options.SessionVerId unconditionally, which also broke the legitimate warm-reconnect path (explicit ReconnectAsync/AlwaysNegotiate) where the in-process outbound counter is deliberately carried across a bumped SessionVerId (Spec_4_7_Retransmit/ReconnectRetransmitTests conformance regression). Narrow the fix to only suppress snapshot resume for the specific #208 scenario: TryColdResumeAsync's recoverable-EstablishReuse-reject fallback, via a one-shot _suppressNextSnapshotResume flag consumed by the very next HydrateFromSnapshotAsync call. This leaves the intentional live-reconnect sequence continuity behavior untouched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/B3.EntryPoint.Client/EntryPointClient.cs | 26 ++++++++++++++------ 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/B3.EntryPoint.Client/EntryPointClient.cs b/src/B3.EntryPoint.Client/EntryPointClient.cs index 6b19bd6..82204a6 100644 --- a/src/B3.EntryPoint.Client/EntryPointClient.cs +++ b/src/B3.EntryPoint.Client/EntryPointClient.cs @@ -29,6 +29,14 @@ public sealed class EntryPointClient : IEntryPointClient, ISubmitOrder, IReplace private readonly Channel _events; private TcpClient? _tcp; private FixpClientSession? _session; + + // #208: set by TryColdResumeAsync immediately before it bumps SessionVerID + // and falls through to a fresh Negotiate (recoverable EstablishReuse + // reject). Consumed exactly once by the following HydrateFromSnapshotAsync + // call so it does NOT resume outbound/inbound state from the now-stale + // (pre-bump) snapshot — a fresh Negotiate starts application sequencing + // over on both sides regardless of the persisted counters. + private bool _suppressNextSnapshotResume; private KeepAliveScheduler? _keepAlive; private RetransmitRequestHandler? _retransmit; private DateTime _lastInboundUtc; @@ -513,14 +521,13 @@ private async ValueTask HydrateFromSnapshotAsync(CancellationToken ct) _options.Logger.StaleSnapshotIgnored(snapshot.SessionId, _options.SessionId); return; } - if (snapshot.SessionVerId != _options.SessionVerId) + if (_suppressNextSnapshotResume) { - // A SessionVerId bump (e.g. cold-start EstablishReuse rejected, - // falling back to a fresh Negotiate; see #208) means this is a - // logically new negotiated session: FIXP resets application-level - // sequence numbering to 1 on both sides regardless of the old - // snapshot's contents, so resuming from it here would silently - // desync the outbound MsgSeqNum from what the venue expects. + _suppressNextSnapshotResume = false; + // See field doc on _suppressNextSnapshotResume (#208): a cold-start + // EstablishReuse reject just bumped SessionVerID and fell through + // to a fresh Negotiate — the snapshot's counters belong to the + // rejected (pre-bump) SessionVerID and must not be resumed. _options.Logger.StaleSnapshotSessionVerIdIgnored(snapshot.SessionVerId, _options.SessionVerId, _options.SessionId); return; } @@ -1596,6 +1603,11 @@ private async Task TryColdResumeAsync(CancellationToken ct) _options.Logger.ColdResumeFallbackToNegotiate(code, prevVerId); _options.SessionVerId = nextVerId; + // #208: the fresh Negotiate about to happen starts application + // sequencing over; the snapshot we just loaded belongs to the + // rejected, pre-bump SessionVerID and must not seed the new session's + // outbound/inbound counters. + _suppressNextSnapshotResume = true; return false; }