diff --git a/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor b/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor
index 81c4a731..e2eee217 100644
--- a/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor
+++ b/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor
@@ -719,8 +719,6 @@
AddStatusMessage($"Changing server to: {newServerUrl}...", MessageType.Info);
await client.ChangeServer(newServerUrl);
-
- CurrentServerUrl = client.Url();
}
catch (Exception ex)
{
@@ -730,6 +728,13 @@
}
finally
{
+ // Read the address back whether or not the connection succeeded. ChangeServer switches
+ // the client's target before it starts connecting, so a switch to a server that is down
+ // throws (the acquisition timeout) while the client is already reconnecting to the NEW
+ // address. Updating this only on success left the label showing the previous server, and
+ // the "Already connected to this server" guard above then compared against a stale value
+ // and refused a legitimate switch back.
+ CurrentServerUrl = client.Url();
IsChangingServer = false;
StateHasChanged();
}
diff --git a/Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs b/Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs
new file mode 100644
index 00000000..7d434a45
--- /dev/null
+++ b/Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs
@@ -0,0 +1,295 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using System;
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+
+using Xrpl.Client;
+
+namespace Xrpl.Tests
+{
+ ///
+ /// Concurrency smoke tests for the reconnect session — the _reconnectCts /
+ /// _reconnectLoop / _reconnectAttempts triple, which is now updated under a
+ /// shared lock.
+ ///
+ ///
+ ///
+ /// These do not reproduce the race the lock fixes. That window is a few instructions
+ /// wide — a start landing between the stop path's cancel, dispose and null — and driving it
+ /// from public API calls, which are separated by whole awaits, does not hit it: with the lock
+ /// removed again these tests still pass. Claiming them as regression coverage would be false.
+ ///
+ ///
+ /// What they do earn their place for is the other direction. Introducing a lock around the
+ /// session creates a deadlock risk of its own: the loop is now started while the lock is held,
+ /// and anything that called back into consumer code from there could re-enter a path that takes
+ /// the same lock. These tests hammer ChangeServer and Disconnect concurrently and require the
+ /// client to still reach a live server afterwards, so a deadlock or a lost session shows up as
+ /// a hang or a failure here rather than in production.
+ ///
+ ///
+ [TestClass]
+ public class TestUReconnectSessionRaces
+ {
+ private CreateMockRippled _mockedRippled;
+ private CreateMockRippled _secondRippled;
+ private XrplClient _client;
+ private int _port;
+
+ private static Dictionary ServerInfoResponse() => new Dictionary
+ {
+ { "type", "response" },
+ { "status", "success" },
+ { "result", new Dictionary
+ {
+ { "info", new Dictionary
+ {
+ { "build_version", "test-mock" },
+ { "complete_ledgers", "1-1" },
+ { "server_state", "full" },
+ }
+ },
+ }
+ },
+ };
+
+ private static CreateMockRippled StartMock(int port)
+ {
+ CreateMockRippled mock = new CreateMockRippled(port) { suppressOutput = true };
+ mock.AddResponse("server_info", ServerInfoResponse());
+
+ // Called directly rather than on a background thread: Start() binds, listens and hands
+ // off to BeginAccept without blocking, so returning from it means the port is already
+ // accepting. Handing it to a thread only opened a window where a test could connect
+ // before the mock was up.
+ mock.Start();
+ return mock;
+ }
+
+ [TestInitialize]
+ public void MyTestInitialize()
+ {
+ _port = TestUtils.GetFreePort();
+ _mockedRippled = StartMock(_port);
+ }
+
+ [TestCleanup]
+ public async Task MyTestCleanup()
+ {
+ if (_client != null)
+ {
+ try
+ {
+ await _client.Disconnect();
+ }
+ catch (Exception)
+ {
+ // The client may already be down; cleanup must not mask the test result.
+ }
+
+ _client = null;
+ }
+
+ _mockedRippled?.Stop();
+ _secondRippled?.Stop();
+ }
+
+ private XrplClient CreateClient(string url) =>
+ new XrplClient(url, new XrplClient.ClientOptions
+ {
+ RequestPolicy = RequestFailurePolicy.ImmediateFail,
+ ReconnectBaseDelay = TimeSpan.FromMilliseconds(50),
+ ReconnectMaxDelay = TimeSpan.FromMilliseconds(400),
+ MaxReconnectAttempts = 100,
+ StopAfterMaxAttempts = false,
+ ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(20),
+ ConnectionAttemptTimeout = TimeSpan.FromSeconds(5),
+ UseCustomPing = false,
+ });
+
+ ///
+ /// Concurrent ChangeServer calls tear down and install reconnect sessions from several
+ /// threads at once. Whatever interleaving wins, the client must end up able to connect to the
+ /// live server — not stranded with a disposed or orphaned session.
+ ///
+ [TestMethod]
+ public async Task TestConcurrentChangeServerKeepsClientRecoverable()
+ {
+ int deadPortA = TestUtils.GetFreePort();
+ int deadPortB = TestUtils.GetFreePort();
+
+ _client = CreateClient($"ws://127.0.0.1:{_port}");
+ await _client.Connect();
+ Assert.IsTrue(_client.connection.IsConnected(), "Precondition: connected to the live mock.");
+
+ // Writers of the reconnect session running at once: two pointed at ports where nothing
+ // listens (each starts a reconnect sequence), one pointed back at the live server, plus a
+ // Disconnect taking the session down underneath them.
+ for (int round = 0; round < 5; round++)
+ {
+ Task[] racers =
+ {
+ SwitchTo($"ws://127.0.0.1:{deadPortA}"),
+ SwitchTo($"ws://127.0.0.1:{deadPortB}"),
+ SwitchTo($"ws://127.0.0.1:{_port}"),
+ Task.Run(async () =>
+ {
+ // Disconnect takes the same session down while the switches install new
+ // ones — the stop-vs-start interleaving the lock has to make safe.
+ try
+ {
+ await _client.Disconnect();
+ }
+ catch (Exception)
+ {
+ }
+ }),
+ };
+
+ await Task.WhenAll(racers);
+ }
+
+ // Whoever won, point the client at the live server and require it to get there.
+ await SwitchTo($"ws://127.0.0.1:{_port}");
+ try { await _client.Connect(); } catch (Exception) { }
+
+ DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30);
+ while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline)
+ {
+ await Task.Delay(TimeSpan.FromMilliseconds(100));
+ }
+
+ Assert.IsTrue(
+ _client.connection.IsConnected(),
+ "After concurrent ChangeServer calls the client could not reach a server that is up — " +
+ "the reconnect session was left disposed or orphaned.");
+ }
+
+ ///
+ /// Disconnect racing a reconnect sequence must leave the client cleanly stopped and
+ /// still able to reconnect afterwards — a stop that tore down someone else's session would
+ /// either strand a live loop or leave a stale one running.
+ ///
+ [TestMethod]
+ public async Task TestDisconnectRacingReconnectLeavesClientReconnectable()
+ {
+ int deadPort = TestUtils.GetFreePort();
+
+ _client = CreateClient($"ws://127.0.0.1:{_port}");
+ await _client.Connect();
+
+ for (int round = 0; round < 5; round++)
+ {
+ // Start a reconnect sequence against a dead port and disconnect while it runs.
+ Task switching = SwitchTo($"ws://127.0.0.1:{deadPort}");
+ Task disconnecting = Task.Run(async () =>
+ {
+ await Task.Delay(TimeSpan.FromMilliseconds(20));
+ await _client.Disconnect();
+ });
+
+ await Task.WhenAll(switching, disconnecting);
+ }
+
+ // The client must still be usable: point it back at the live server and connect.
+ // Both calls are tolerated so the assertion below reports the failure, rather than the
+ // test dying on a raw exception from a switch that lost a race.
+ await SwitchTo($"ws://127.0.0.1:{_port}");
+ try
+ {
+ await _client.Connect();
+ }
+ catch (Exception)
+ {
+ }
+
+ DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30);
+ while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline)
+ {
+ await Task.Delay(TimeSpan.FromMilliseconds(100));
+ }
+
+ Assert.IsTrue(
+ _client.connection.IsConnected(),
+ "Disconnect racing a reconnect sequence left the client unable to connect again.");
+ }
+
+ ///
+ /// A failed Connect issued while a reconnect loop is already running must leave a
+ /// live loop behind, so the client still comes back on its own once the server returns.
+ ///
+ ///
+ /// Covers the functional path end to end. It does not pin the narrow race that made
+ /// StopReconnectLoop drop the loop reference: that needs the retired task to still be
+ /// running when the restart checks IsCompleted, and by the time a failed Connect gets
+ /// there the task has normally already exited, so the loop is restarted either way — with
+ /// the fix reverted this test still passes. Kept because the path itself (Connect while
+ /// reconnecting, server appears later) is worth guarding.
+ ///
+ [TestMethod]
+ public async Task TestFailedConnectDuringReconnectLeavesLoopRunning()
+ {
+ int laterPort = TestUtils.GetFreePort();
+
+ // Short acquisition timeout: the Connect below is expected to fail, and waiting out the
+ // class default would add 20s of nothing to the run.
+ _client = new XrplClient($"ws://127.0.0.1:{_port}", new XrplClient.ClientOptions
+ {
+ RequestPolicy = RequestFailurePolicy.ImmediateFail,
+ ReconnectBaseDelay = TimeSpan.FromMilliseconds(50),
+ ReconnectMaxDelay = TimeSpan.FromMilliseconds(400),
+ MaxReconnectAttempts = 100,
+ StopAfterMaxAttempts = false,
+ ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(3),
+ ConnectionAttemptTimeout = TimeSpan.FromSeconds(2),
+ UseCustomPing = false,
+ });
+ await _client.Connect();
+ Assert.IsTrue(_client.connection.IsConnected(), "Precondition: connected to the live mock.");
+
+ // Point the client at a port where nothing listens: a reconnect loop starts and retries.
+ await SwitchTo($"ws://127.0.0.1:{laterPort}");
+ Assert.IsFalse(_client.connection.IsConnected(), "Precondition: the target port is closed.");
+
+ // A Connect while that loop is running: it stops the loop, then fails because nothing
+ // is listening yet. Something must still be reconnecting afterwards.
+ try
+ {
+ await _client.Connect();
+ }
+ catch (Exception)
+ {
+ // Expected - nothing is listening on that port yet.
+ }
+
+ // The server appears. Nobody touches the client from here on.
+ _secondRippled = StartMock(laterPort);
+
+ DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(40);
+ while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline)
+ {
+ await Task.Delay(TimeSpan.FromMilliseconds(200));
+ }
+
+ Assert.IsTrue(
+ _client.connection.IsConnected(),
+ "The client never reconnected after the server returned - a failed Connect during " +
+ "an active reconnect sequence left no loop running.");
+ }
+
+ private async Task SwitchTo(string url)
+ {
+ try
+ {
+ await _client.connection.ChangeServer(url);
+ }
+ catch (Exception)
+ {
+ // Failing to reach a dead port is the point of the race; the invariant is asserted
+ // by the caller once the dust settles.
+ }
+ }
+ }
+}
diff --git a/Xrpl/Client/connection.cs b/Xrpl/Client/connection.cs
index 2362bc6d..261bfdf0 100644
--- a/Xrpl/Client/connection.cs
+++ b/Xrpl/Client/connection.cs
@@ -287,10 +287,37 @@ private static WebSocketClient CreateWebSocket(string url, ConnectionOptions con
private static readonly Random _random = new();
- // Volatile: StopReconnectLoop, StartReconnectLoop and RetireCurrentSessionAndReconnectAsync
- // write this from other threads, and the loop compares it by reference to decide whether it
- // still owns the reconnect state. A stale read would let a retired loop run one more
- // iteration, or make the owning loop stand down. Matches the other cross-thread fields here.
+ ///
+ /// Guards the reconnect session — , and
+ /// — wherever one is read and another written as a unit:
+ /// , ,
+ /// , and
+ /// the ownership-guarded writes in .
+ ///
+ ///
+ /// Not every touch of these fields is covered: the per-iteration _reconnectAttempts++ in
+ /// , and the plain resets in ChangeServer and
+ /// OnceClose, still run outside it. Those predate this lock; do not read the list above
+ /// as "all three fields are always synchronized".
+ ///
+ ///
+ ///
+ /// volatile alone was not enough: it makes each individual access atomic, not the
+ /// sequence of them. The stop path used to read the field three times in a row (Cancel,
+ /// Dispose, null it), so a start running in between could have its brand-new source disposed
+ /// and cleared by the retiring stop — leaving the loop with a dead source and nobody
+ /// reconnecting, which is exactly the permanent wedge this whole area exists to prevent.
+ ///
+ ///
+ /// Nothing that can call back into consumer code runs while the lock is held: cancellation and
+ /// disposal of a retired source happen after the lock is released, and the loop body starts
+ /// with a yield so that starting it under the lock never runs a notification inline.
+ ///
+ ///
+ private readonly object _reconnectStateLock = new object();
+
+ // Volatile so the ownership checks in ReconnectLoopAsync can read it outside the lock:
+ // a single reference read is atomic, and those checks only ever compare, never mutate.
private volatile CancellationTokenSource _reconnectCts;
private Task _reconnectLoop;
@@ -541,15 +568,20 @@ private async Task RetireCurrentSessionAndReconnectAsync(string reason)
_reconnectMode = ReconnectMode.FastReconnect;
_isFastReconnectActive = true; // Keep for backward compatibility
- // 2. Stop any existing reconnect loop
- var oldCts = _reconnectCts;
+ // 2-3. Retire the previous reconnect session and install this one as a single transaction,
+ // so a concurrent stop/start cannot dispose the source created here. Cancellation and
+ // disposal of the old source happen after the lock is released.
+ CancellationTokenSource oldCts;
+ lock (_reconnectStateLock)
+ {
+ oldCts = _reconnectCts;
+ _reconnectLoop = null; // Clear old loop reference so StartReconnectLoop can start a new one
+ _reconnectAttempts = 1;
+ _reconnectCts = new CancellationTokenSource();
+ }
+
oldCts?.Cancel();
oldCts?.Dispose();
- _reconnectLoop = null; // Clear old loop reference so StartReconnectLoop can start a new one
-
- // 3. Initialize reconnect state BEFORE any notifications
- _reconnectAttempts = 1;
- _reconnectCts = new CancellationTokenSource();
// 4. Now send first notification - IsReconnectActive() will return true
SetConnectionState(
@@ -631,10 +663,17 @@ private async Task RetireCurrentSessionAndReconnectAsync(string reason)
// Connect succeeded - cleanup reconnect state
// Note: _reconnectMode will be cleared in OnceOpen when connection is fully established
_isFastReconnectActive = false;
- _reconnectCts?.Cancel();
- _reconnectCts?.Dispose();
- _reconnectCts = null;
- _reconnectAttempts = 0;
+
+ CancellationTokenSource settled;
+ lock (_reconnectStateLock)
+ {
+ settled = _reconnectCts;
+ _reconnectCts = null;
+ _reconnectAttempts = 0;
+ }
+
+ settled?.Cancel();
+ settled?.Dispose();
}
catch (Exception ex)
{
@@ -1797,9 +1836,38 @@ await errorHandler
// the backoff at ReconnectBaseDelay. With StopAfterMaxAttempts = false (no give-up branch)
// that means connect -> handler failure -> teardown forever at a constant 2s, a sustained
// connection load on a node that accepts TCP but cannot serve requests yet.
- StopReconnectLoop();
- _reconnectLoop = null;
- StartReconnectLoop(initialAttempts: failures);
+ RestartReconnectLoop(initialAttempts: failures);
+ }
+
+ ///
+ /// Retires the current reconnect session and installs a fresh one in a single transaction,
+ /// seeding the attempt counter with .
+ ///
+ ///
+ /// Doing this as StopReconnectLoop(); _reconnectLoop = null; StartReconnectLoop(seed);
+ /// took the lock twice with a bare write in between, so a concurrent start (from OnceClose or
+ /// OnConnectionFailed) could slip in and install its own loop; the seeded start would then see
+ /// a live loop, return without applying the seed, and the backoff would silently stop growing
+ /// across consecutive handler failures — the very regression the seed exists to prevent.
+ ///
+ private void RestartReconnectLoop(int initialAttempts)
+ {
+ CancellationTokenSource retired;
+ lock (_reconnectStateLock)
+ {
+ retired = _reconnectCts;
+ _reconnectMode = ReconnectMode.LoopReconnect;
+ _isFastReconnectActive = false;
+ _reconnectAttempts = initialAttempts;
+ _reconnectCts = new CancellationTokenSource();
+
+ // Safe to start under the lock: ReconnectLoopAsync reads its token and yields before
+ // anything else, so this only schedules the loop - no consumer notification runs inline.
+ _reconnectLoop = ReconnectLoopAsync(_reconnectCts);
+ }
+
+ retired?.Cancel();
+ retired?.Dispose();
}
private async Task OnceClose(int? code, string? description, WebSocketClient closingSocket, long sessionId)
@@ -1942,10 +2010,27 @@ private async Task OnceClose(int? code, string? description, WebSocketClient clo
private void StopReconnectLoop()
{
- _reconnectCts?.Cancel();
- _reconnectCts?.Dispose();
- _reconnectCts = null;
- _reconnectAttempts = 0;
+ // Detach under the lock, then cancel/dispose outside it: a start racing with this stop can
+ // no longer have its fresh source torn down, and cancellation callbacks never run while the
+ // lock is held.
+ CancellationTokenSource retired;
+ lock (_reconnectStateLock)
+ {
+ retired = _reconnectCts;
+ _reconnectCts = null;
+ _reconnectAttempts = 0;
+
+ // Drop the task reference too, in the same transaction. The retired loop exits
+ // asynchronously - it only notices it lost ownership on its next check - so leaving the
+ // reference behind makes StartReconnectLoop see `!IsCompleted` and return without
+ // starting anything, while the retired loop then stands down on its ownership check.
+ // Nobody would be reconnecting. Reachable whenever Connect or ChangeServer stops a live
+ // loop and the new connection fails.
+ _reconnectLoop = null;
+ }
+
+ retired?.Cancel();
+ retired?.Dispose();
// Note: Do NOT clear _reconnectMode here!
// _reconnectMode is cleared only by:
// - OnceOpen (connection succeeded)
@@ -1976,37 +2061,49 @@ private void StartReconnectLoop(int initialAttempts = 0)
{
// Set reconnect mode to LoopReconnect (upgrades from FastReconnect or sets from None)
_reconnectMode = ReconnectMode.LoopReconnect;
-
- // CRITICAL: If a loop is already running, don't start another or reset the counter
- // This prevents _reconnectAttempts from being reset mid-loop when callbacks trigger
- // reconnect logic (OnceClose, OnConnectionFailed, etc.)
- var loopIsRunning = _reconnectLoop != null && !_reconnectLoop.IsCompleted;
- if (loopIsRunning)
- {
- // Loop is already running - let it continue, don't reset _reconnectAttempts
- return;
- }
-
- // If we have a valid pre-created CTS (from RetireCurrentSessionAndReconnectAsync),
- // we should reuse it. Check for this case first.
- var existingCts = _reconnectCts;
- var hasValidPreCreatedCts = existingCts != null && !existingCts.IsCancellationRequested;
-
- // If no valid pre-created CTS, create a new one
- // Only reset _reconnectAttempts when creating a FRESH CTS (new reconnect sequence)
- if (!hasValidPreCreatedCts)
+
+ // The whole decision — is a loop already running, is the current source reusable, install a
+ // fresh one, hand it to the new loop — is one transaction. Split across the lock it would
+ // race with StopReconnectLoop and with another start: two loops could end up running, or a
+ // loop could be handed a source that a concurrent stop has already disposed.
+ CancellationTokenSource retired = null;
+ lock (_reconnectStateLock)
{
- // Cancel/dispose old CTS if any
- existingCts?.Cancel();
- existingCts?.Dispose();
- _reconnectCts = new CancellationTokenSource();
- _reconnectAttempts = initialAttempts;
+ // CRITICAL: If a loop is already running, don't start another or reset the counter
+ // This prevents _reconnectAttempts from being reset mid-loop when callbacks trigger
+ // reconnect logic (OnceClose, OnConnectionFailed, etc.)
+ var loopIsRunning = _reconnectLoop != null && !_reconnectLoop.IsCompleted;
+ if (loopIsRunning)
+ {
+ // Loop is already running - let it continue, don't reset _reconnectAttempts
+ return;
+ }
+
+ // If we have a valid pre-created CTS (from RetireCurrentSessionAndReconnectAsync),
+ // we should reuse it. Check for this case first.
+ var existingCts = _reconnectCts;
+ var hasValidPreCreatedCts = existingCts != null && !existingCts.IsCancellationRequested;
+
+ // If no valid pre-created CTS, create a new one
+ // Only reset _reconnectAttempts when creating a FRESH CTS (new reconnect sequence)
+ if (!hasValidPreCreatedCts)
+ {
+ // Retire the old CTS after the lock is released - see _reconnectStateLock
+ retired = existingCts;
+ _reconnectCts = new CancellationTokenSource();
+ _reconnectAttempts = initialAttempts;
+ }
+ // else: Reuse existing valid CTS (pre-created for fast reconnect)
+ // Don't reset _reconnectAttempts - this is continuation of existing reconnect sequence
+ // Note: _reconnectLoop was already cleared by RetireCurrentSessionAndReconnectAsync
+
+ // Safe to start under the lock: ReconnectLoopAsync yields before touching anything, so
+ // this call only schedules the loop and returns - no consumer notification runs inline.
+ _reconnectLoop = ReconnectLoopAsync(_reconnectCts);
}
- // else: Reuse existing valid CTS (pre-created for fast reconnect)
- // Don't reset _reconnectAttempts - this is continuation of existing reconnect sequence
- // Note: _reconnectLoop was already cleared by RetireCurrentSessionAndReconnectAsync
-
- _reconnectLoop = ReconnectLoopAsync(_reconnectCts);
+
+ retired?.Cancel();
+ retired?.Dispose();
}
private async Task ReconnectLoopAsync(CancellationTokenSource ownCts)
@@ -2014,8 +2111,20 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts)
// The CTS this loop owns. StopReconnectLoop cancels without awaiting the loop, so a retired loop
// can still be running - or reach its tail - after a replacement has been installed. Everything
// this loop writes to shared reconnect state is therefore guarded by an ownership check.
+ //
+ // Read BEFORE the yield below, and deliberately so: the caller still holds
+ // _reconnectStateLock here, so this source cannot yet have been retired. After the yield a
+ // concurrent stop may already have disposed it - Cancel/Dispose of a retired source run
+ // outside the lock - and CancellationTokenSource.Token throws ObjectDisposedException once
+ // disposed. Taken after the yield, that throw would land outside every try below, faulting
+ // the loop before its first attempt and vanishing as an unobserved task exception.
CancellationToken ct = ownCts.Token;
+ // Yield so nothing beyond that read runs inline on the caller: StartReconnectLoop starts the
+ // loop while holding _reconnectStateLock, and a consumer notification executing under that
+ // lock could deadlock against any path that takes it (Disconnect from a handler, say).
+ await Task.Yield();
+
// Don't reset _reconnectAttempts here - it may be pre-set to 1 by fast reconnect path
// StartReconnectLoop() sets it to 0 when creating a new CTS
@@ -2078,6 +2187,14 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts)
{
break;
}
+ catch (ObjectDisposedException)
+ {
+ // The source this loop owns was retired and disposed while the delay was being
+ // set up: registering a callback on a token whose source is gone throws instead
+ // of cancelling. Same meaning as cancellation - a newer sequence owns the
+ // reconnect state now - so leave quietly rather than fault the task.
+ break;
+ }
}
if (ct.IsCancellationRequested)
@@ -2118,9 +2235,14 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts)
if (IsConnected())
{
- if (ReferenceEquals(_reconnectCts, ownCts))
+ // Ownership check and the write it guards belong together: checked outside the
+ // lock, this loop could be retired in between and reset a live sequence's counter.
+ lock (_reconnectStateLock)
{
- _reconnectAttempts = 0;
+ if (ReferenceEquals(_reconnectCts, ownCts))
+ {
+ _reconnectAttempts = 0;
+ }
}
break;
@@ -2170,8 +2292,19 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts)
if (config.StopAfterMaxAttempts && _reconnectAttempts >= config.MaxReconnectAttempts)
{
- _reconnectCts?.Dispose();
- _reconnectCts = null;
+ // Re-check ownership inside the lock: between the check above and here a new sequence
+ // could have installed its own source, and disposing that one would strand it.
+ CancellationTokenSource finished = null;
+ lock (_reconnectStateLock)
+ {
+ if (ReferenceEquals(_reconnectCts, ownCts))
+ {
+ finished = _reconnectCts;
+ _reconnectCts = null;
+ }
+ }
+
+ finished?.Dispose();
}
}