Release 11.3.2.0 - #180
Merged
Merged
Conversation
…sts (#178) * fix(connection): retire the session before sweeping the pending requests A request created while the client was retiring its connection was written to the socket being retired and hung until RequestTimeout (issue #177). Every retirement path swept the pending requests with RejectAllWithCancellation() and only afterwards cleared `ws`. The sweep resumes consumer continuations - inline on the sweeping thread when there is no synchronization context, since RequestManager's completion sources are not created with RunContinuationsAsynchronously - and a consumer that issued its next request from there passed ShouldBeConnected() on the old socket, was sent into it after the sweep, and was never completed. Moving the clear ahead of the first await, as the issue suggested, does not close that window: the continuation runs inside the sweep itself. Clear `ws` and mark the session retiring before the sweep on all five paths: ChangeServer, the ping/network fast reconnect, Disconnect, DisconnectAndWaitAsync, and the failed-OnConnected-handler path, which the issue did not list. `ws` is now volatile - it is the gate every request reads lock-free. A DisconnectedException out of the send no longer leaves the just-created request pending for RequestTimeout; it is rejected like an encoding failure and surfaces through the same await. The regression test pins the ordering with an ExecuteSynchronously continuation, which is the shape the sweep hands a consumer on a thread pool: it asserts the socket the follow-up saw is not the retired one, and that it completes within seconds rather than after RequestTimeout. * fix(connection): let the retirement paths yield instead of block, and settle a fast reconnect once Three defects seen on the Blazor WebAssembly stand while verifying #177, all on the paths that retire a session. Stopping the message processor blocked the calling thread on the reader task for up to two seconds. On a single-threaded host the reader's continuation needs that very thread to observe the completed channel, so the block never returned early: every ChangeServer, fast reconnect and Disconnect stalled the UI for the full two seconds. The stop is now detach-under-lock plus an awaited exit with the same cap, taken after the request sweep so consumers are released first. Measured on the stand: 2000 ms to 5-164 ms per switch. RetireCurrentSessionAndReconnectAsync is awaited from inside the ping check, and only from there, yet it waited for the ping to finish - its own - and so ran out WaitForPingToFinishAsync's three-second timeout on every ping-triggered reconnect. An AsyncLocal set by the ping check lets the wait recognise the ping it runs in. RestoringConnection to the session-ended notification: 6 s to 20 ms. When the fast reconnect's own attempt failed at the socket, the failure callback started the reconnect loop on the same cancellation source; the loop connected first, OnceOpen retired the source, and the fast reconnect's wait came back cancelled. Its catch read that as a failure: it reported RestoringConnection on a connected client and started a second loop, whose first attempt retired the live socket and opened another. A connected client is now recognised as settled in that catch, and the loop itself no longer retires a socket that is open when its turn comes. Both timing defects and the double reconnect are pinned by tests on a silent-on-ping server: the first measures the retirement gap, the second takes the server down at RestoringConnection so the sequence falls to the loop, brings a replacement up on the same port, and requires exactly one connection afterwards. * chore(release): Xrpl 11.3.2.0 Patch: the connection fixes for #177 and the three defects found while verifying it change no contract. Only the Xrpl package moves - the base packages are untouched since 11.3.1.0 and keep their published versions. * fix(connection): scope the ping-check marker to its connection, and let the switch tests fail as themselves Review follow-ups on #178. The AsyncLocal that lets WaitForPingToFinishAsync recognise the ping it runs in was static. Its value follows the execution context, so a consumer's OnPing handler that awaited another connection would carry the flag into that connection and let it skip waiting for its own ping. It is an instance field now. The regression tests judged the follow-up request inside a try whose catch-all also caught the assertion failures, replacing the original message with an exception-type mismatch. The outcome is captured first and judged afterwards. The IsNotNull on the response went with it: XrplResponse is a struct, and completion within the bound is all a success has to show. * fix(connection): a socket a user Disconnect() took cannot come back, and a stale handler failure keeps its hands off the live connection Review follow-ups on #178, all on code adjacent to the retirement paths. OnceOpen checked only the session. Disconnect() does not retire the session - OnceClose is what announces a user disconnect - so a handshake that completed while Disconnect() was running had its OnConnect callback install the socket, clear the intentional-disconnect tracking, and hand the close that followed to the reconnect loop as a network drop. The socket's own marks are the signal now: a socket Disconnect() took, or a client that is permanently disconnected, is refused. OnConnectHandlerFailedAsync swept the pending requests, stopped the message processor and the ping timer before asking whether the failed socket was still the current one. When a newer connection had replaced it, that cleanup hit the newer connection. The ownership check comes first; a stale callback closes its own socket and steps aside. DisconnectAndWaitAsync returned "already disconnected" at once when a concurrent call had taken the socket, while that call was still closing it. It now waits on the disconnect in progress, within its own timeout. * fix(connection): a stale OnConnected failure is not the current connection's failure Review follow-up on #178. The ownership check in OnConnectHandlerFailedAsync sat after the give-up branch, so a delayed failure from a socket that a newer connection had already replaced still counted towards giving up and, with StopAfterMaxAttempts on, ran RejectAll and Disconnect() against the live connection. The check comes first now, read-only: a stale callback closes its own socket and returns before anything is counted or torn down. The clearing check under the same lock stays where it was, for a replacement that lands during the consumer notifications in between. * fix(connection): the give-up teardown rechecks ownership after notifying Review follow-up on #178. In OnConnectHandlerFailedAsync the give-up branch notifies the consumer before it rejects the requests and calls Disconnect() - deliberately, so the reason reaches the consumer before Disconnect() moves the state itself. That notification runs consumer code: a handler that answers "gave up" with a ChangeServer has already taken the failed socket out of ws and is opening another, and the teardown would have hit that connection. Ownership is rechecked after the notification; the three checks in the method now share one helper. * fix(connection): the handler-failure notification goes out before the socket is taken Review follow-up on #178. Moving the RestoringConnection notification behind the clearing ownership check, as the previous round did, left the consumer's handler running after this path had already taken the socket out of ws: a ChangeServer from that handler installed a replacement, and the cleanup that followed - ping timer, sweep, processor, reconnect state - hit it, with no way to tell from ws alone. The notification now precedes the clear, so the ownership check that comes with the clear is the one that sees what the handler did: a ChangeServer retires the socket itself, the check finds it gone, and this path closes its socket and steps aside.
Platonenkov
enabled auto-merge
September 6, 2026 23:36
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.
Promotes
devtorelease, publishingXrpl11.3.2.0 to nuget.org. One squashed commit on top of 11.3.1.0: #178, closing #177.What ships
A request issued while the client is switching servers no longer hangs until
RequestTimeout(#177). Every path that retires a connection swept the pending requests first and cleared the socket reference afterwards. The sweep resumes the consumer, and a consumer that issues its next request from there - the second value of a page load, read from the response handler of the first - found the retired socket still installed, passed the connectivity check on it, and was written into it after the sweep that would have rejected it. Forty seconds later it timed out, with the connection healthy for thirty-nine of them.The window is not where the issue placed it.
RequestManagerbuilds its completion sources withoutRunContinuationsAsynchronously, so on a thread pool the consumer's continuation runs inline, inside the sweep itself, before anyawait- moving the clear ahead of the firstawait, as proposed, leaves the regression test red. The socket reference is cleared before the sweep on all five retirement paths, one of which the issue did not list.Three defects found on the Blazor WebAssembly stand while verifying it, each reproduced there and pinned by a test that was red before the fix:
ChangeServer, fast reconnect andDisconnectstalled a single-threaded host for two seconds - a blocking wait on the stream reader whose continuation needed the very thread that was blocked. Awaited now: 2000 ms → 5–390 ms per switch;RestoringConnectionon a connected client and started a second loop that retired the live socket. Consumers saw twoOnConnectedper recovery and restored their subscriptions twice.From the review, five more windows on the same paths closed: a socket a user
Disconnect()took can no longer be installed by a lateOnConnect; a staleOnConnected-handler failure no longer tears down the connection that replaced it, counts towards giving up, or gives up on its behalf; a concurrentDisconnectAndWaitAsyncwaits for the disconnect in progress instead of reporting one that has not finished.Versions
Only
Xrplmoves, 11.3.1.0 → 11.3.2.0.Xrpl.AddressCodec(10.9.0.0),Xrpl.BinaryCodec(11.0.1.0) andXrpl.Keypairs(10.9.0.0) stay:git diff origin/release...origin/dev -- Base/is empty. Patch: no contract changes.Worth knowing before merging
The failure a consumer sees changes type. A request issued during a switch used to fail after 40 s with
TimeoutException; withRequestPolicy.ImmediateFailit now fails at once withNotConnectedException, and withWaitForConnectionit is carried over to the new connection. Retry logic that recognised the old failure by its type or message needs to know the new one - the changelog says so, and the consuming projects have issues for it.NotConnectedException()on that path carries the runtime's default message; giving it a real one is in #179.One
OnConnectedper recovery, not two. Code that de-duplicated subscription restores on its own keeps working; code that relied on the second callback for anything was relying on a bug.The retirement paths now yield where they used to block. On a single-threaded host, consumer code can run during a
ChangeServerorDisconnectat the point the stream reader is let go. Re-entrancy guards a consumer already needed around the connect that follows now matter oneawaitearlier.Not in this release, collected in #179: the ordering of two concurrent user commands (
ChangeServervsDisconnect), the reconnect loop's ownership hand-off toOnceClose, and the no-awaitwindow between reading the socket and sending on it.Verification
Xrpl.wasmchecked against the sourceaccount_tx;docker pause→ ping retire → unpause inside the in-flight attempt → oneConnected;docker stop→ 1005 → loop →docker start→ recovered; switch to an unreachable address and back over a live loopintegrationgreen on the merge group (34063342810)fixCleanup3_4_0remains open and untouched by this release.