From a8277d01577517ec37dc97bbdf6744c29a3386fa Mon Sep 17 00:00:00 2001 From: Paul Medynski <31868385+paulmedynski@users.noreply.github.com> Date: Thu, 7 May 2026 13:50:46 -0300 Subject: [PATCH 1/4] Harden SqlConnection state transitions with CAS guards --- .../Microsoft/Data/SqlClient/SqlConnection.cs | 64 +++- .../SqlConnectionStateTransitionTests.cs | 279 ++++++++++++++++++ 2 files changed, 337 insertions(+), 6 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs index 6a4d7b8972..f603d520ff 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -2224,7 +2224,16 @@ private bool TryOpen(TaskCompletionSource retry, SqlConnec return result; } - private bool TryOpenInner(TaskCompletionSource retry) + /// + /// Completes the inner open/replace operation and initializes parser state for the active inner connection. + /// + /// Retry continuation used by async open paths. + /// when open initialization completed synchronously; otherwise . + /// + /// The inner connection is snapshotted after the open call so downstream parser access uses a single observed + /// instance and does not rely on a second racy read of . + /// + internal bool TryOpenInner(TaskCompletionSource retry) { // Create a single TimeoutTimer that represents the overall connection-open budget for this // attempt. The timer is threaded through every layer (inner connection state transitions, @@ -2396,11 +2405,32 @@ internal void RemoveWeakReference(object value) // ClosedBusy->Closed (never opened) // Connecting->Closed (exception during open, return to previous closed state) + /// + /// Finalizes a non-evented transition from a transitional inner state to a stable closed state. + /// + /// The target inner connection state object. + /// + /// This path is restricted to transition sources that intentionally serialize external callers while state is + /// being updated. A compare-and-swap prevents stale writers from overwriting a newer inner connection state. + /// internal void SetInnerConnectionTo(DbConnectionInternal to) { Debug.Assert(_innerConnection != null, "null InnerConnection"); Debug.Assert(to != null, "to null InnerConnection"); - _innerConnection = to; + + DbConnectionInternal originalInnerConnection = _innerConnection; + if (originalInnerConnection != DbConnectionClosedBusy.SingletonInstance && + originalInnerConnection != DbConnectionClosedConnecting.SingletonInstance) + { + // SetInnerConnectionTo is only valid while leaving a known transitional state. + throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + } + + if (originalInnerConnection != Interlocked.CompareExchange(ref _innerConnection, to, originalInnerConnection)) + { + // Reject stale writers when another thread already advanced the state machine. + throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + } } // Closed->Connecting: prevent set_ConnectionString during Open @@ -2418,14 +2448,37 @@ internal bool SetInnerConnectionFrom(DbConnectionInternal to, DbConnectionIntern // OpenBusy->Closed (previously opened) // Connecting->Open + /// + /// Finalizes an evented state transition from a transitional inner state to a stable target state. + /// + /// The target inner connection state object. + /// + /// This path is intentionally restricted to the expected transition sources ( + /// and ). A compare-and-swap enforces that the transition commits only + /// if no concurrent writer has already changed . + /// internal void SetInnerConnectionEvent(DbConnectionInternal to) { Debug.Assert(_innerConnection != null, "null InnerConnection"); Debug.Assert(to != null, "to null InnerConnection"); - ConnectionState originalState = _innerConnection.State & ConnectionState.Open; + DbConnectionInternal originalInnerConnection = _innerConnection; + if (originalInnerConnection != DbConnectionOpenBusy.SingletonInstance && + originalInnerConnection != DbConnectionClosedConnecting.SingletonInstance) + { + // Evented transitions are valid only from the two expected transitional sources. + throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + } + + ConnectionState originalState = originalInnerConnection.State & ConnectionState.Open; ConnectionState currentState = to.State & ConnectionState.Open; + if (originalInnerConnection != Interlocked.CompareExchange(ref _innerConnection, to, originalInnerConnection)) + { + // Ensure event and close-count side effects are tied to a committed transition only. + throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + } + if ((originalState != currentState) && (ConnectionState.Closed == currentState)) { unchecked @@ -2434,8 +2487,6 @@ internal void SetInnerConnectionEvent(DbConnectionInternal to) } } - _innerConnection = to; - if (ConnectionState.Closed == originalState && ConnectionState.Open == currentState) { OnStateChange(DbConnectionInternal.StateChangeOpen); @@ -2562,9 +2613,10 @@ private void ConnectionString_Set(ConnectionPoolKey key) flag = SetInnerConnectionFrom(DbConnectionClosedBusy.SingletonInstance, connectionInternal); if (flag) { + // Publish option/pool metadata before releasing the busy state back to a stable closed state. _userConnectionOptions = connectionOptions; _poolGroup = poolGroup; - _innerConnection = DbConnectionClosedNeverOpened.SingletonInstance; + SetInnerConnectionTo(DbConnectionClosedNeverOpened.SingletonInstance); } } if (!flag) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs new file mode 100644 index 0000000000..24124bc4c7 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs @@ -0,0 +1,279 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Data; +using System.Data.Common; +using System.Threading.Tasks; +using System.Transactions; +using Microsoft.Data.ProviderBase; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests; + +/// +/// Regression tests for connection state transitions that can be affected by concurrent state updates. +/// +public class SqlConnectionStateTransitionTests +{ + /// + /// Verifies TryOpenInner does not surface an invalid cast when the inner state changes after TryOpenConnection. + /// + [Fact] + public void TryOpenInner_WhenInnerConnectionChangesAfterTryOpen_ThrowsInvalidOperation() + { + using SqlConnection connection = new(); + connection.ForceNewConnection = false; + bool initialized = connection.SetInnerConnectionFrom(new TestRaceDbConnectionInternal(), DbConnectionClosedNeverOpened.SingletonInstance); + Assert.True(initialized); + + InvalidOperationException exception = Assert.Throws(() => connection.TryOpenInner(null)); + Assert.NotNull(exception); + } + + /// + /// Verifies TryOpenInner does not surface an invalid cast when the inner state changes after TryReplaceConnection. + /// + [Fact] + public void TryOpenInner_WhenInnerConnectionChangesAfterTryReplace_ThrowsInvalidOperation() + { + using SqlConnection connection = new(); + connection.ForceNewConnection = true; + bool initialized = connection.SetInnerConnectionFrom(new TestRaceDbConnectionInternal(), DbConnectionClosedNeverOpened.SingletonInstance); + Assert.True(initialized); + + InvalidOperationException exception = Assert.Throws(() => connection.TryOpenInner(null)); + Assert.NotNull(exception); + } + + /// + /// Verifies evented state transitions reject non-transitional source states. + /// + [Fact] + public void SetInnerConnectionEvent_WhenInnerConnectionIsNotTransitionState_ThrowsInvalidOperation() + { + using SqlConnection connection = new(); + bool initialized = connection.SetInnerConnectionFrom(DbConnectionClosedPreviouslyOpened.SingletonInstance, DbConnectionClosedNeverOpened.SingletonInstance); + Assert.True(initialized); + + int initialCloseCount = connection.CloseCount; + int eventCount = 0; + connection.StateChange += (_, _) => eventCount++; + + InvalidOperationException exception = Assert.Throws( + () => connection.SetInnerConnectionEvent(new TestOpenDbConnectionInternal())); + Assert.NotNull(exception); + Assert.Equal(initialCloseCount, connection.CloseCount); + Assert.Equal(0, eventCount); + } + + /// + /// Verifies direct transition-to-state writes reject non-transitional source states. + /// + [Fact] + public void SetInnerConnectionTo_WhenInnerConnectionIsNotTransitionState_ThrowsInvalidOperation() + { + using SqlConnection connection = new(); + bool initialized = connection.SetInnerConnectionFrom(DbConnectionClosedPreviouslyOpened.SingletonInstance, DbConnectionClosedNeverOpened.SingletonInstance); + Assert.True(initialized); + + InvalidOperationException exception = Assert.Throws( + () => connection.SetInnerConnectionTo(DbConnectionClosedNeverOpened.SingletonInstance)); + Assert.NotNull(exception); + } + + /// + /// Verifies Connecting-to-Open event transitions raise a single open state-change notification. + /// + [Fact] + public void SetInnerConnectionEvent_FromConnectingToOpen_RaisesOpenStateChange() + { + using SqlConnection connection = new(); + bool enteredConnecting = connection.SetInnerConnectionFrom(DbConnectionClosedConnecting.SingletonInstance, DbConnectionClosedNeverOpened.SingletonInstance); + Assert.True(enteredConnecting); + + int initialCloseCount = connection.CloseCount; + StateChangeEventArgs? stateChange = null; + connection.StateChange += (_, e) => stateChange = e; + + connection.SetInnerConnectionEvent(new TestOpenDbConnectionInternal()); + + Assert.NotNull(stateChange); + Assert.Equal(ConnectionState.Closed, stateChange!.OriginalState); + Assert.Equal(ConnectionState.Open, stateChange.CurrentState); + Assert.Equal(initialCloseCount, connection.CloseCount); + } + + /// + /// Verifies OpenBusy-to-Closed event transitions raise a single closed state-change notification and increment close count. + /// + [Fact] + public void SetInnerConnectionEvent_FromOpenBusyToClosed_RaisesClosedStateChangeAndIncrementsCloseCount() + { + using SqlConnection connection = new(); + TestOpenDbConnectionInternal openConnection = new(); + + bool enteredOpen = connection.SetInnerConnectionFrom(openConnection, DbConnectionClosedNeverOpened.SingletonInstance); + Assert.True(enteredOpen); + bool enteredOpenBusy = connection.SetInnerConnectionFrom(DbConnectionOpenBusy.SingletonInstance, openConnection); + Assert.True(enteredOpenBusy); + + int initialCloseCount = connection.CloseCount; + StateChangeEventArgs? stateChange = null; + connection.StateChange += (_, e) => stateChange = e; + + connection.SetInnerConnectionEvent(DbConnectionClosedPreviouslyOpened.SingletonInstance); + + Assert.NotNull(stateChange); + Assert.Equal(ConnectionState.Open, stateChange!.OriginalState); + Assert.Equal(ConnectionState.Closed, stateChange.CurrentState); + Assert.Equal(initialCloseCount + 1, connection.CloseCount); + } + + /// + /// A test inner connection that forces a post-open state change to simulate race-sensitive transition windows. + /// + private sealed class TestRaceDbConnectionInternal : DbConnectionInternal + { + /// + /// Initializes a closed test connection that permits connection-string updates. + /// + internal TestRaceDbConnectionInternal() : base(ConnectionState.Closed, true, true) + { + } + + /// + /// Gets a placeholder server version for abstract member satisfaction. + /// + public override string ServerVersion => "0"; + + /// + /// Not supported in this test double. + /// + /// Requested isolation level. + /// Never returns; always throws. + public override DbTransaction BeginTransaction(System.Data.IsolationLevel il) => throw new NotSupportedException(); + + /// + /// No-op for this test double. + /// + /// Transaction to enlist. + public override void EnlistTransaction(Transaction transaction) + { + } + + /// + /// No-op reset for this test double. + /// + internal override void ResetConnection() + { + } + + /// + /// No-op activation for this test double. + /// + /// Transaction supplied by the caller. + protected override void Activate(Transaction transaction) + { + } + + /// + /// No-op deactivation for this test double. + /// + protected override void Deactivate() + { + } + + /// + /// Simulates a race by replacing the inner connection with a closed singleton after open succeeds. + /// + /// Owning outer connection. + /// Factory used to mutate inner state. + /// Retry continuation. + /// User connection options. + /// Always . + internal override bool TryOpenConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + SqlConnectionOptions userOptions) + { + connectionFactory.SetInnerConnectionTo(outerConnection, DbConnectionClosedPreviouslyOpened.SingletonInstance); + return true; + } + + /// + /// Simulates a race by replacing the inner connection with a closed singleton after replace succeeds. + /// + /// Owning outer connection. + /// Factory used to mutate inner state. + /// Retry continuation. + /// User connection options. + /// Always . + internal override bool TryReplaceConnection( + DbConnection outerConnection, + SqlConnectionFactory connectionFactory, + TaskCompletionSource retry, + SqlConnectionOptions userOptions) + { + connectionFactory.SetInnerConnectionTo(outerConnection, DbConnectionClosedPreviouslyOpened.SingletonInstance); + return true; + } + } + + /// + /// A minimal open-state inner connection used as a non-transitional target in transition validation tests. + /// + private sealed class TestOpenDbConnectionInternal : DbConnectionInternal + { + /// + /// Initializes an open test connection instance. + /// + internal TestOpenDbConnectionInternal() : base(ConnectionState.Open, true, false) + { + } + + /// + /// Gets a placeholder server version for abstract member satisfaction. + /// + public override string ServerVersion => "0"; + + /// + /// Not supported in this test double. + /// + /// Requested isolation level. + /// Never returns; always throws. + public override DbTransaction BeginTransaction(System.Data.IsolationLevel il) => throw new NotSupportedException(); + + /// + /// No-op for this test double. + /// + /// Transaction to enlist. + public override void EnlistTransaction(Transaction transaction) + { + } + + /// + /// No-op reset for this test double. + /// + internal override void ResetConnection() + { + } + + /// + /// No-op activation for this test double. + /// + /// Transaction supplied by the caller. + protected override void Activate(Transaction transaction) + { + } + + /// + /// No-op deactivation for this test double. + /// + protected override void Deactivate() + { + } + } +} From 0622b0cc7c55bc893fdf269356f8572764538b54 Mon Sep 17 00:00:00 2001 From: Paul Medynski <31868385+paulmedynski@users.noreply.github.com> Date: Thu, 7 May 2026 15:08:30 -0300 Subject: [PATCH 2/4] Refine transition diagnostics and race-state tests --- .../src/Microsoft/Data/Common/AdapterUtil.cs | 4 ++++ .../src/Microsoft/Data/SqlClient/SqlConnection.cs | 8 ++++---- .../SqlClient/SqlConnectionStateTransitionTests.cs | 14 ++++++++------ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index 568592072d..16b2689730 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -1106,6 +1106,10 @@ internal enum ConnectionError GetConnectionReturnsNull, ConnectionOptionsMissing, CouldNotSwitchToClosedPreviouslyOpenedState, + InvalidSourceStateForSetInnerConnectionTo, + FailedToCommitSetInnerConnectionTo, + InvalidSourceStateForSetInnerConnectionEvent, + FailedToCommitSetInnerConnectionEvent, } internal static Exception InternalConnectionError(ConnectionError internalError) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs index f603d520ff..72057762a9 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -2423,13 +2423,13 @@ internal void SetInnerConnectionTo(DbConnectionInternal to) originalInnerConnection != DbConnectionClosedConnecting.SingletonInstance) { // SetInnerConnectionTo is only valid while leaving a known transitional state. - throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + throw ADP.InternalConnectionError(ADP.ConnectionError.InvalidSourceStateForSetInnerConnectionTo); } if (originalInnerConnection != Interlocked.CompareExchange(ref _innerConnection, to, originalInnerConnection)) { // Reject stale writers when another thread already advanced the state machine. - throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + throw ADP.InternalConnectionError(ADP.ConnectionError.FailedToCommitSetInnerConnectionTo); } } @@ -2467,7 +2467,7 @@ internal void SetInnerConnectionEvent(DbConnectionInternal to) originalInnerConnection != DbConnectionClosedConnecting.SingletonInstance) { // Evented transitions are valid only from the two expected transitional sources. - throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + throw ADP.InternalConnectionError(ADP.ConnectionError.InvalidSourceStateForSetInnerConnectionEvent); } ConnectionState originalState = originalInnerConnection.State & ConnectionState.Open; @@ -2476,7 +2476,7 @@ internal void SetInnerConnectionEvent(DbConnectionInternal to) if (originalInnerConnection != Interlocked.CompareExchange(ref _innerConnection, to, originalInnerConnection)) { // Ensure event and close-count side effects are tied to a committed transition only. - throw ADP.InternalConnectionError(ADP.ConnectionError.CouldNotSwitchToClosedPreviouslyOpenedState); + throw ADP.InternalConnectionError(ADP.ConnectionError.FailedToCommitSetInnerConnectionEvent); } if ((originalState != currentState) && (ConnectionState.Closed == currentState)) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs index 24124bc4c7..ccf5d8b416 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs @@ -30,6 +30,7 @@ public void TryOpenInner_WhenInnerConnectionChangesAfterTryOpen_ThrowsInvalidOpe InvalidOperationException exception = Assert.Throws(() => connection.TryOpenInner(null)); Assert.NotNull(exception); + Assert.Same(DbConnectionClosedPreviouslyOpened.SingletonInstance, connection.InnerConnection); } /// @@ -45,6 +46,7 @@ public void TryOpenInner_WhenInnerConnectionChangesAfterTryReplace_ThrowsInvalid InvalidOperationException exception = Assert.Throws(() => connection.TryOpenInner(null)); Assert.NotNull(exception); + Assert.Same(DbConnectionClosedPreviouslyOpened.SingletonInstance, connection.InnerConnection); } /// @@ -191,14 +193,14 @@ protected override void Deactivate() /// Owning outer connection. /// Factory used to mutate inner state. /// Retry continuation. - /// User connection options. /// Always . internal override bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry, - SqlConnectionOptions userOptions) + TaskCompletionSource retry) { + // Mirror the real open path by committing through a valid transitional source state first. + connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this); connectionFactory.SetInnerConnectionTo(outerConnection, DbConnectionClosedPreviouslyOpened.SingletonInstance); return true; } @@ -209,14 +211,14 @@ internal override bool TryOpenConnection( /// Owning outer connection. /// Factory used to mutate inner state. /// Retry continuation. - /// User connection options. /// Always . internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry, - SqlConnectionOptions userOptions) + TaskCompletionSource retry) { + // Mirror the real replace path by committing through a valid transitional source state first. + connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedBusy.SingletonInstance, this); connectionFactory.SetInnerConnectionTo(outerConnection, DbConnectionClosedPreviouslyOpened.SingletonInstance); return true; } From 600b19e5f6f588702573f956c910c2cb0c09c887 Mon Sep 17 00:00:00 2001 From: Paul Medynski <31868385+paulmedynski@users.noreply.github.com> Date: Wed, 10 Jun 2026 12:39:57 -0300 Subject: [PATCH 3/4] Address PR #4267 review feedback: assert exact event count in StateChange tests - Add eventCount tracking to both SetInnerConnectionEvent tests to verify exactly one StateChange event is raised per transition. - Fix TryOpenConnection/TryReplaceConnection override signatures to include TimeoutTimer parameter added on main. --- .../SqlConnectionStateTransitionTests.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs index ccf5d8b416..b1decd2be0 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionStateTransitionTests.cs @@ -96,11 +96,13 @@ public void SetInnerConnectionEvent_FromConnectingToOpen_RaisesOpenStateChange() Assert.True(enteredConnecting); int initialCloseCount = connection.CloseCount; + int eventCount = 0; StateChangeEventArgs? stateChange = null; - connection.StateChange += (_, e) => stateChange = e; + connection.StateChange += (_, e) => { eventCount++; stateChange = e; }; connection.SetInnerConnectionEvent(new TestOpenDbConnectionInternal()); + Assert.Equal(1, eventCount); Assert.NotNull(stateChange); Assert.Equal(ConnectionState.Closed, stateChange!.OriginalState); Assert.Equal(ConnectionState.Open, stateChange.CurrentState); @@ -122,11 +124,13 @@ public void SetInnerConnectionEvent_FromOpenBusyToClosed_RaisesClosedStateChange Assert.True(enteredOpenBusy); int initialCloseCount = connection.CloseCount; + int eventCount = 0; StateChangeEventArgs? stateChange = null; - connection.StateChange += (_, e) => stateChange = e; + connection.StateChange += (_, e) => { eventCount++; stateChange = e; }; connection.SetInnerConnectionEvent(DbConnectionClosedPreviouslyOpened.SingletonInstance); + Assert.Equal(1, eventCount); Assert.NotNull(stateChange); Assert.Equal(ConnectionState.Open, stateChange!.OriginalState); Assert.Equal(ConnectionState.Closed, stateChange.CurrentState); @@ -193,11 +197,13 @@ protected override void Deactivate() /// Owning outer connection. /// Factory used to mutate inner state. /// Retry continuation. + /// Timeout timer (unused in test). /// Always . internal override bool TryOpenConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { // Mirror the real open path by committing through a valid transitional source state first. connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedConnecting.SingletonInstance, this); @@ -211,11 +217,13 @@ internal override bool TryOpenConnection( /// Owning outer connection. /// Factory used to mutate inner state. /// Retry continuation. + /// Timeout timer (unused in test). /// Always . internal override bool TryReplaceConnection( DbConnection outerConnection, SqlConnectionFactory connectionFactory, - TaskCompletionSource retry) + TaskCompletionSource retry, + TimeoutTimer timeout) { // Mirror the real replace path by committing through a valid transitional source state first. connectionFactory.SetInnerConnectionFrom(outerConnection, DbConnectionClosedBusy.SingletonInstance, this); From e740289f06d4023a04095fc82c6c851769a7a77b Mon Sep 17 00:00:00 2001 From: Paul Medynski <31868385+paulmedynski@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:12:57 -0300 Subject: [PATCH 4/4] Fix ConcurrentOpenTests to use SetInnerConnectionFrom for CAS-guarded transitions The CAS guard in SetInnerConnectionTo rejects writes when the current state is not a valid transitional state (DbConnectionClosedBusy or DbConnectionClosedConnecting). Tests from ef6aacfa9 (main) assumed plain assignment. Use SetInnerConnectionFrom instead, which performs a CAS swap from the current state without precondition validation on the source state. --- .../Data/SqlClient/SqlConnectionConcurrentOpenTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionConcurrentOpenTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionConcurrentOpenTests.cs index c38a30d317..8d151e61cd 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionConcurrentOpenTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlConnectionConcurrentOpenTests.cs @@ -36,7 +36,11 @@ private static DbConnectionInternal GetConnectingSingleton() private static void ForceInnerConnection(SqlConnection connection, DbConnectionInternal innerConnectionValue) { - connection.SetInnerConnectionTo(innerConnectionValue); + // Use SetInnerConnectionFrom to transition from the current state via CAS. + // SetInnerConnectionTo requires the connection to already be in a transitional state. + DbConnectionInternal current = connection.InnerConnection; + bool swapped = connection.SetInnerConnectionFrom(innerConnectionValue, current); + Assert.True(swapped, $"CAS failed: expected current state {current.GetType().Name} but it changed concurrently."); } private static bool InvokeTryOpenInner(SqlConnection connection, TaskCompletionSource retry)