Skip to content

Commit 71f4b6c

Browse files
authored
ConnectionState: Add thread safety with atomic updates (#284)
1 parent 6a2c3b7 commit 71f4b6c

File tree

2 files changed

+19
-6
lines changed

2 files changed

+19
-6
lines changed

Source/HiveMQtt/Client/Connection/ConnectionManager.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,18 @@ public partial class ConnectionManager : IDisposable
4444
// This is how we kill innocent and not so innocent Tasks
4545
private CancellationTokenSource cancellationTokenSource;
4646

47-
// The state of the connection
48-
internal ConnectState State { get; set; }
47+
// The state of the connection (thread-safe using Interlocked)
48+
private int stateValue = (int)ConnectState.Disconnected;
49+
50+
/// <summary>
51+
/// Gets or sets the connection state in a thread-safe manner.
52+
/// Uses Volatile.Read for reads and Interlocked.Exchange for writes to ensure thread safety.
53+
/// </summary>
54+
internal ConnectState State
55+
{
56+
get => (ConnectState)Volatile.Read(ref this.stateValue);
57+
set => Interlocked.Exchange(ref this.stateValue, (int)value);
58+
}
4959

5060
// The protocol specific transport layer (TCP, WebSocket, etc.)
5161
internal BaseTransport Transport { get; set; }

Source/HiveMQtt/Client/Connection/ConnectionManagerHandlers.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ internal async Task HandleIncomingPubCompPacketAsync(PubCompPacket pubCompPacket
380380
/// <returns>A task that represents the asynchronous operation.</returns>
381381
internal async Task<bool> HandleDisconnectionAsync(bool clean = true)
382382
{
383+
// Thread-safe check: if already disconnected, return early
383384
if (this.State == ConnectState.Disconnected)
384385
{
385386
Logger.Trace("HandleDisconnection: Already disconnected.");
@@ -388,18 +389,20 @@ internal async Task<bool> HandleDisconnectionAsync(bool clean = true)
388389

389390
Logger.Debug($"HandleDisconnection: Handling disconnection. clean={clean}.");
390391

391-
// Cancel all background tasks and close the socket
392-
this.State = ConnectState.Disconnected;
393-
394392
// Reset the connection-ready signal for the next connect cycle
395393
this.ResetConnectedSignal();
396394

397-
// Cancel all background tasks
395+
// Cancel all background tasks BEFORE setting state to Disconnected
396+
// This prevents race conditions where tasks check state after it's set but before cancellation
398397
await this.CancelBackgroundTasksAsync().ConfigureAwait(false);
399398

400399
// Close the Transport
401400
await this.Transport.CloseAsync().ConfigureAwait(false);
402401

402+
// Set state to Disconnected AFTER tasks are cancelled and transport is closed
403+
// This ensures tasks see the correct state when they check during cancellation
404+
this.State = ConnectState.Disconnected;
405+
403406
if (clean)
404407
{
405408
if (!this.SendQueue.IsEmpty)

0 commit comments

Comments
 (0)