From fce8b208a29a5f712fef25277a390de805975b46 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 5 Aug 2025 14:50:31 -0500 Subject: [PATCH 01/16] Public ctors --- .../Data/SqlClient/SqlCommand.netcore.cs | 40 --------------- .../Data/SqlClient/SqlCommand.netfx.cs | 42 ---------------- .../Microsoft/Data/SqlClient/SqlCommand.cs | 50 ++++++++++++++++++- 3 files changed, 49 insertions(+), 83 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 1de6ca10b5..2048526b26 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -6,7 +6,6 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; -using System.ComponentModel; using System.Data; using System.Data.Common; using System.Data.SqlTypes; @@ -16,11 +15,8 @@ using System.Text; using System.Threading; using System.Threading.Tasks; -using System.Xml; using Microsoft.Data.Common; -using Microsoft.Data.Sql; using Microsoft.Data.SqlClient.Diagnostics; -using Microsoft.Data.SqlClient.Server; // NOTE: The current Microsoft.VSDesigner editor attributes are implemented for System.Data.SqlClient, and are not publicly available. // New attributes that are designed to work with Microsoft.Data.SqlClient and are publicly documented should be included in future. @@ -246,42 +242,6 @@ private AsyncState CachedAsyncState /// internal bool CachingQueryMetadataPostponed { get; set; } - /// - public SqlCommand() : base() - { - GC.SuppressFinalize(this); - } - - /// - public SqlCommand(string cmdText) : this() - { - CommandText = cmdText; - } - - /// - public SqlCommand(string cmdText, SqlConnection connection) : this() - { - CommandText = cmdText; - Connection = connection; - } - - /// - public SqlCommand(string cmdText, SqlConnection connection, SqlTransaction transaction) : this() - { - CommandText = cmdText; - Connection = connection; - Transaction = transaction; - } - - /// - public SqlCommand(string cmdText, SqlConnection connection, SqlTransaction transaction, SqlCommandColumnEncryptionSetting columnEncryptionSetting) : this() - { - CommandText = cmdText; - Connection = connection; - Transaction = transaction; - _columnEncryptionSetting = columnEncryptionSetting; - } - private SqlCommand(SqlCommand from) : this() { CommandText = from.CommandText; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index 28e9b1ab60..45e86c9771 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; -using System.ComponentModel; using System.Data; using System.Data.Common; using System.Data.SqlTypes; @@ -17,12 +16,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; -using System.Xml; -using System.Buffers; using Microsoft.Data.Common; -using Microsoft.Data.Sql; -using Microsoft.Data.SqlClient.Server; -using System.Transactions; using System.Collections.Concurrent; // NOTE: The current Microsoft.VSDesigner editor attributes are implemented for System.Data.SqlClient, and are not publicly available. @@ -245,42 +239,6 @@ private AsyncState CachedAsyncState /// internal bool CachingQueryMetadataPostponed { get; set; } - /// - public SqlCommand() : base() - { - GC.SuppressFinalize(this); - } - - /// - public SqlCommand(string cmdText) : this() - { - CommandText = cmdText; - } - - /// - public SqlCommand(string cmdText, SqlConnection connection) : this() - { - CommandText = cmdText; - Connection = connection; - } - - /// - public SqlCommand(string cmdText, SqlConnection connection, SqlTransaction transaction) : this() - { - CommandText = cmdText; - Connection = connection; - Transaction = transaction; - } - - /// - public SqlCommand(string cmdText, SqlConnection connection, SqlTransaction transaction, SqlCommandColumnEncryptionSetting columnEncryptionSetting) : this() - { - CommandText = cmdText; - Connection = connection; - Transaction = transaction; - _columnEncryptionSetting = columnEncryptionSetting; - } - private SqlCommand(SqlCommand from) : this() { CommandText = from.CommandText; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index ef39945ea4..742412587c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -195,6 +195,54 @@ public sealed partial class SqlCommand : DbCommand, ICloneable #endregion + #region Constructors + + /// + public SqlCommand() + { + GC.SuppressFinalize(this); + } + + /// + public SqlCommand(string cmdText) + : this() + { + CommandText = cmdText; + } + + /// + public SqlCommand(string cmdText, SqlConnection connection) + : this() + { + CommandText = cmdText; + Connection = connection; + } + + /// + public SqlCommand(string cmdText, SqlConnection connection, SqlTransaction transaction) + : this() + { + CommandText = cmdText; + Connection = connection; + Transaction = transaction; + } + + /// + public SqlCommand( + string cmdText, + SqlConnection connection, + SqlTransaction transaction, + SqlCommandColumnEncryptionSetting columnEncryptionSetting) + : this() + { + CommandText = cmdText; + Connection = connection; + Transaction = transaction; + _columnEncryptionSetting = columnEncryptionSetting; + } + + #endregion + #region Events /// @@ -236,7 +284,7 @@ private enum EXECTYPE } #endregion - + #region Public Properties /// From 76e910d3ae8fd42008dbe945dfc069bb18df680f Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 5 Aug 2025 15:02:02 -0500 Subject: [PATCH 02/16] Merge Clone and Clone ctor --- .../Data/SqlClient/SqlCommand.netcore.cs | 29 --------------- .../Data/SqlClient/SqlCommand.netfx.cs | 29 --------------- .../Microsoft/Data/SqlClient/SqlCommand.cs | 37 +++++++++++++++++++ 3 files changed, 37 insertions(+), 58 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 2048526b26..d6c98037a2 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -242,24 +242,6 @@ private AsyncState CachedAsyncState /// internal bool CachingQueryMetadataPostponed { get; set; } - private SqlCommand(SqlCommand from) : this() - { - CommandText = from.CommandText; - CommandTimeout = from.CommandTimeout; - CommandType = from.CommandType; - Connection = from.Connection; - DesignTimeVisible = from.DesignTimeVisible; - Transaction = from.Transaction; - UpdatedRowSource = from.UpdatedRowSource; - _columnEncryptionSetting = from.ColumnEncryptionSetting; - - SqlParameterCollection parameters = Parameters; - foreach (object parameter in from.Parameters) - { - parameters.Add((parameter is ICloneable) ? (parameter as ICloneable).Clone() : parameter); - } - } - private bool IsProviderRetriable => SqlConfigurableRetryFactory.IsRetriable(RetryLogicProvider); /// @@ -2241,17 +2223,6 @@ private void ReadDescribeEncryptionParameterResults( } } - /// - public SqlCommand Clone() - { - SqlCommand clone = new SqlCommand(this); - SqlClientEventSource.Log.TryTraceEvent("SqlCommand.Clone | API | Object Id {0}, Clone Object Id {1}, Client Connection Id {2}", ObjectID, clone.ObjectID, Connection?.ClientConnectionId); - return clone; - } - - object ICloneable.Clone() => - Clone(); - private Task RegisterForConnectionCloseNotification(Task outerTask) { SqlConnection connection = _activeConnection; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index 45e86c9771..aaec69f3e4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -239,24 +239,6 @@ private AsyncState CachedAsyncState /// internal bool CachingQueryMetadataPostponed { get; set; } - private SqlCommand(SqlCommand from) : this() - { - CommandText = from.CommandText; - CommandTimeout = from.CommandTimeout; - CommandType = from.CommandType; - Connection = from.Connection; - DesignTimeVisible = from.DesignTimeVisible; - Transaction = from.Transaction; - UpdatedRowSource = from.UpdatedRowSource; - _columnEncryptionSetting = from.ColumnEncryptionSetting; - - SqlParameterCollection parameters = Parameters; - foreach (object parameter in from.Parameters) - { - parameters.Add((parameter is ICloneable) ? (parameter as ICloneable).Clone() : parameter); - } - } - private bool IsProviderRetriable => SqlConfigurableRetryFactory.IsRetriable(RetryLogicProvider); /// @@ -2203,17 +2185,6 @@ private void ReadDescribeEncryptionParameterResults( } } - /// - public SqlCommand Clone() - { - SqlCommand clone = new SqlCommand(this); - SqlClientEventSource.Log.TryTraceEvent("SqlCommand.Clone | API | Object Id {0}, Clone Object Id {1}, Client Connection Id {2}", ObjectID, clone.ObjectID, Connection?.ClientConnectionId); - return clone; - } - - object ICloneable.Clone() => - Clone(); - private Task RegisterForConnectionCloseNotification(Task outterTask) { SqlConnection connection = _activeConnection; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 742412587c..abdee411ad 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -241,6 +241,27 @@ public SqlCommand( _columnEncryptionSetting = columnEncryptionSetting; } + private SqlCommand(SqlCommand from) + { + CommandText = from.CommandText; + CommandTimeout = from.CommandTimeout; + CommandType = from.CommandType; + Connection = from.Connection; + DesignTimeVisible = from.DesignTimeVisible; + Transaction = from.Transaction; + UpdatedRowSource = from.UpdatedRowSource; + _columnEncryptionSetting = from.ColumnEncryptionSetting; + + SqlParameterCollection parameters = Parameters; + foreach (object parameter in from.Parameters) + { + object parameterToAdd = parameter is ICloneable cloneableParameter + ? cloneableParameter.Clone() + : parameter; + parameters.Add(parameterToAdd); + } + } + #endregion #region Events @@ -743,6 +764,22 @@ private bool IsDirty #region Public/Internal Methods + object ICloneable.Clone() => + Clone(); + + /// + public SqlCommand Clone() + { + SqlCommand clone = new SqlCommand(this); + SqlClientEventSource.Log.TryTraceEvent( + "SqlCommand.Clone | API | " + + $"Object Id {ObjectID}, " + + $"Clone Object Id {clone.ObjectID}, " + + $"Client Connection Id {_activeConnection?.ClientConnectionId}"); + + return clone; + } + /// public override void Prepare() { From 60e4c2aa28c448ef6885473b13c7f9051288622c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Tue, 5 Aug 2025 17:22:35 -0500 Subject: [PATCH 03/16] Merge ResetCommandTimeout --- .../Microsoft/Data/SqlClient/SqlCommand.netcore.cs | 10 ---------- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 13 +++++++++++++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index d6c98037a2..bdfab58df5 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -244,16 +244,6 @@ private AsyncState CachedAsyncState private bool IsProviderRetriable => SqlConfigurableRetryFactory.IsRetriable(RetryLogicProvider); - /// - public void ResetCommandTimeout() - { - if (ADP.DefaultCommandTimeout != CommandTimeout) - { - PropertyChanging(); - _commandTimeout = DefaultCommandTimeout; - } - } - internal void OnStatementCompleted(int recordCount) { if (0 <= recordCount) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index abdee411ad..bb4cf66abf 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -859,6 +859,19 @@ public override void Prepare() } } + #if NET + // @TODO: Why not just expose this for netfx? + /// + public void ResetCommandTimeout() + { + if (CommandTimeout != ADP.DefaultCommandTimeout) + { + PropertyChanging(); + _commandTimeout = DefaultCommandTimeout; + } + } + #endif + #endregion #region Private Methods From a3262f6f8a17e4fee378377f5c98fa56ca460ea9 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:19:19 -0500 Subject: [PATCH 04/16] Merge Cancel # Conflicts: # src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs --- .../Data/SqlClient/SqlCommand.netcore.cs | 92 ------------------ .../Data/SqlClient/SqlCommand.netfx.cs | 93 ------------------ .../Microsoft/Data/SqlClient/SqlCommand.cs | 96 +++++++++++++++++++ 3 files changed, 96 insertions(+), 185 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index bdfab58df5..9f9e17c0b6 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -267,98 +267,6 @@ internal void OnStatementCompleted(int recordCount) } } - // Cancel is supposed to be multi-thread safe. - // It doesn't make sense to verify the connection exists or that it is open during cancel - // because immediately after checking the connection can be closed or removed via another thread. - // - /// - public override void Cancel() - { - using (TryEventScope.Create("SqlCommand.Cancel | API | Object Id {0}", ObjectID)) - { - SqlClientEventSource.Log.TryCorrelationTraceEvent("SqlCommand.Cancel | API | Correlation | Object Id {0}, Activity Id {1}, Client Connection Id {2}, Command Text '{3}'", ObjectID, ActivityCorrelator.Current, Connection?.ClientConnectionId, CommandText); - - SqlStatistics statistics = null; - try - { - statistics = SqlStatistics.StartTimer(Statistics); - - // If we are in reconnect phase simply cancel the waiting task - var reconnectCompletionSource = _reconnectionCompletionSource; - if (reconnectCompletionSource != null) - { - if (reconnectCompletionSource.TrySetCanceled()) - { - return; - } - } - - // the pending data flag means that we are awaiting a response or are in the middle of processing a response - // if we have no pending data, then there is nothing to cancel - // if we have pending data, but it is not a result of this command, then we don't cancel either. Note that - // this model is implementable because we only allow one active command at any one time. This code - // will have to change we allow multiple outstanding batches - if (_activeConnection == null) - { - return; - } - SqlInternalConnectionTds connection = (_activeConnection.InnerConnection as SqlInternalConnectionTds); - if (connection == null) - { // Fail with out locking - return; - } - - // The lock here is to protect against the command.cancel / connection.close race condition - // The SqlInternalConnectionTds is set to OpenBusy during close, once this happens the cast below will fail and - // the command will no longer be cancelable. It might be desirable to be able to cancel the close operation, but this is - // outside of the scope of Whidbey RTM. See (SqlConnection::Close) for other lock. - lock (connection) - { - if (connection != (_activeConnection.InnerConnection as SqlInternalConnectionTds)) - { - // make sure the connection held on the active connection is what we have stored in our temp connection variable, if not between getting "connection" and taking the lock, the connection has been closed - return; - } - - TdsParser parser = connection.Parser; - if (parser == null) - { - return; - } - - if (!_pendingCancel) - { - // Do nothing if already pending. - // Before attempting actual cancel, set the _pendingCancel flag to false. - // This denotes to other thread before obtaining stateObject from the - // session pool that there is another thread wishing to cancel. - // The period in question is between entering the ExecuteAPI and obtaining - // a stateObject. - _pendingCancel = true; - - TdsParserStateObject stateObj = _stateObj; - if (stateObj != null) - { - stateObj.Cancel(this); - } - else - { - SqlDataReader reader = connection.FindLiveReader(this); - if (reader != null) - { - reader.Cancel(this); - } - } - } - } - } - finally - { - SqlStatistics.StopTimer(statistics); - } - } - } - /// new public SqlParameter CreateParameter() { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index aaec69f3e4..6479b1ae59 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -276,99 +276,6 @@ internal void OnStatementCompleted(int recordCount) } } - // Cancel is supposed to be multi-thread safe. - // It doesn't make sense to verify the connection exists or that it is open during cancel - // because immediately after checkin the connection can be closed or removed via another thread. - // - /// - public override void Cancel() - { - using (TryEventScope.Create("SqlCommand.Cancel | API | Object Id {0}", ObjectID)) - { - SqlClientEventSource.Log.TryCorrelationTraceEvent("SqlCommand.Cancel | API | Correlation | Object Id {0}, Activity Id {1}, Client Connection Id {2}, Command Text '{3}'", ObjectID, ActivityCorrelator.Current, Connection?.ClientConnectionId, CommandText); - - SqlStatistics statistics = null; - try - { - statistics = SqlStatistics.StartTimer(Statistics); - - // If we are in reconnect phase simply cancel the waiting task - var reconnectCompletionSource = _reconnectionCompletionSource; - if (reconnectCompletionSource != null) - { - if (reconnectCompletionSource.TrySetCanceled()) - { - return; - } - } - - // the pending data flag means that we are awaiting a response or are in the middle of processing a response - // if we have no pending data, then there is nothing to cancel - // if we have pending data, but it is not a result of this command, then we don't cancel either. Note that - // this model is implementable because we only allow one active command at any one time. This code - // will have to change we allow multiple outstanding batches - if (_activeConnection == null) - { - return; - } - SqlInternalConnectionTds connection = (_activeConnection.InnerConnection as SqlInternalConnectionTds); - if (connection == null) - { // Fail with out locking - return; - } - - // The lock here is to protect against the command.cancel / connection.close race condition - // The SqlInternalConnectionTds is set to OpenBusy during close, once this happens the cast below will fail and - // the command will no longer be cancelable. It might be desirable to be able to cancel the close operation, but this is - // outside of the scope of Whidbey RTM. See (SqlConnection::Close) for other lock. - lock (connection) - { - if (connection != (_activeConnection.InnerConnection as SqlInternalConnectionTds)) - { - // make sure the connection held on the active connection is what we have stored in our temp connection variable, if not between getting "connection" and taking the lock, the connection has been closed - return; - } - - TdsParser parser = connection.Parser; - if (parser == null) - { - return; - } - - if (!_pendingCancel) - { - // Do nothing if aleady pending. - // Before attempting actual cancel, set the _pendingCancel flag to false. - // This denotes to other thread before obtaining stateObject from the - // session pool that there is another thread wishing to cancel. - // The period in question is between entering the ExecuteAPI and obtaining - // a stateObject. - _pendingCancel = true; - - TdsParserStateObject stateObj = _stateObj; - if (stateObj != null) - { - stateObj.Cancel(this); - } - else - { - SqlDataReader reader = connection.FindLiveReader(this); - if (reader != null) - { - reader.Cancel(this); - } - } - } - // @TODO: CER Exception Handling was removed here (see GH#3581) - } - } - finally - { - SqlStatistics.StopTimer(statistics); - } - } - } - /// new public SqlParameter CreateParameter() { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index bb4cf66abf..098998d3e1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -767,6 +767,102 @@ private bool IsDirty object ICloneable.Clone() => Clone(); + /// + public override void Cancel() + { + // Cancel is supposed to be multi-thread safe. + // It doesn't make sense to verify the connection exists or that it is open during + // cancel because immediately after checking the connection can be closed or removed + // via another thread. + + using var eventScope = TryEventScope.Create($"SqlCommand.Cancel | API | Object Id {ObjectID}"); + SqlClientEventSource.Log.TryCorrelationTraceEvent( + "SqlCommand.Cancel | API | Correlation | " + + $"Object Id {ObjectID}, " + + $"Activity Id {ActivityCorrelator.Current}, " + + $"Client Connection Id {_activeConnection.ClientConnectionId}, " + + $"Command Text '{CommandText}'"); + + SqlStatistics statistics = null; + try + { + statistics = SqlStatistics.StartTimer(Statistics); + + // If we are in reconnect phase simply cancel the waiting task + var reconnectCompletionSource = _reconnectionCompletionSource; + if (reconnectCompletionSource is not null && reconnectCompletionSource.TrySetCanceled()) + { + return; + } + + // The pending data flag means that we are awaiting a response or are in the middle + // of processing a response. + // * If we have no pending data, then there is nothing to cancel. + // * If we have pending data, but it is not a result of this command, then we don't + // cancel either. + // Note that this model is implementable because we only allow one active command + // at any one time. This code will have to change we allow multiple outstanding + // batches. + if (_activeConnection?.InnerConnection is not SqlInternalConnectionTds connection) + { + // @TODO: Really this case only applies if the connection is null. + // Fail without locking + return; + } + + // The lock here is to protect against the command.cancel / connection.close race + // condition. The SqlInternalConnectionTds is set to OpenBusy during close, once + // this happens the cast below will fail and the command will no longer be + // cancelable. It might be desirable to be able to cancel the close operation, but + // this is outside the scope of Whidbey RTM. See (SqlConnection::Close) for other lock. + lock (connection) + { + // Make sure the connection did not get changed getting the connection and + // taking the lock. If it has, the connection has been closed. + if (connection != _activeConnection.InnerConnection as SqlInternalConnectionTds) + { + return; + } + + TdsParser parser = connection.Parser; + if (parser is null) + { + return; + } + + if (!_pendingCancel) + { + // Do nothing if already pending. + // Before attempting actual cancel, set the _pendingCancel flag to false. + // This denotes to other thread before obtaining stateObject from the + // session pool that there is another thread wishing to cancel. + // The period in question is between entering the ExecuteAPI and obtaining + // a stateObject. + _pendingCancel = true; + + TdsParserStateObject stateObj = _stateObj; + if (stateObj is not null) + { + stateObj.Cancel(this); + } + else + { + SqlDataReader reader = connection.FindLiveReader(this); + if (reader is not null) + { + reader.Cancel(this); + } + } + } + } + // @TODO: CER Exception Handling was removed here (see GH#3581) + } + finally + { + SqlStatistics.StopTimer(statistics); + } + } + /// public SqlCommand Clone() { From 930064971453f10440a682caf57cf6d1da529235 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:21:57 -0500 Subject: [PATCH 05/16] Merge CreateParameter --- .../src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs | 6 ------ .../netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 6 ------ .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 4 ++++ 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 9f9e17c0b6..64331ecd78 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -267,12 +267,6 @@ internal void OnStatementCompleted(int recordCount) } } - /// - new public SqlParameter CreateParameter() - { - return new SqlParameter(); - } - /// protected override DbParameter CreateDbParameter() { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index 6479b1ae59..ad08447b79 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -276,12 +276,6 @@ internal void OnStatementCompleted(int recordCount) } } - /// - new public SqlParameter CreateParameter() - { - return new SqlParameter(); - } - /// protected override DbParameter CreateDbParameter() { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 098998d3e1..986c60acd4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -876,6 +876,10 @@ public SqlCommand Clone() return clone; } + /// + public new SqlParameter CreateParameter() => + new SqlParameter(); + /// public override void Prepare() { From cd5514596cd341f93099d755f7674f11cd66bbd8 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:23:53 -0500 Subject: [PATCH 06/16] Merge CreateDbParameter --- .../src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs | 6 ------ .../src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 6 ------ .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 8 ++++++++ 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 64331ecd78..aada6588a4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -267,12 +267,6 @@ internal void OnStatementCompleted(int recordCount) } } - /// - protected override DbParameter CreateDbParameter() - { - return CreateParameter(); - } - /// protected override void Dispose(bool disposing) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index ad08447b79..cfa94daec9 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -276,12 +276,6 @@ internal void OnStatementCompleted(int recordCount) } } - /// - protected override DbParameter CreateDbParameter() - { - return CreateParameter(); - } - /// protected override void Dispose(bool disposing) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 986c60acd4..8645d7a46c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -974,6 +974,14 @@ public void ResetCommandTimeout() #endregion + #region Protected Methods + + /// + protected override DbParameter CreateDbParameter() => + CreateParameter(); + + #endregion + #region Private Methods // @TODO: Rename to PrepareInternal From 390aff1cac8d3f48dfb0ede561cb443a205bbe6a Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:25:44 -0500 Subject: [PATCH 07/16] Merge Dispose --- .../Data/SqlClient/SqlCommand.netcore.cs | 15 --------------- .../Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 15 --------------- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index aada6588a4..6388172c8d 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -267,21 +267,6 @@ internal void OnStatementCompleted(int recordCount) } } - /// - protected override void Dispose(bool disposing) - { - if (disposing) - { - // release managed objects - _cachedMetaData = null; - - // reset async cache information to allow a second async execute - CachedAsyncState?.ResetAsyncState(); - } - // release unmanaged objects - base.Dispose(disposing); - } - private void VerifyEndExecuteState(Task completionTask, string endMethod, bool fullCheckForColumnEncryption = false) { Debug.Assert(completionTask != null); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index cfa94daec9..e21cb94795 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -276,21 +276,6 @@ internal void OnStatementCompleted(int recordCount) } } - /// - protected override void Dispose(bool disposing) - { - if (disposing) - { - // release managed objects - _cachedMetaData = null; - - // reset async cache information to allow a second async execute - CachedAsyncState?.ResetAsyncState(); - } - // release unmanaged objects - base.Dispose(disposing); - } - private void VerifyEndExecuteState(Task completionTask, string endMethod, bool fullCheckForColumnEncryption = false) { Debug.Assert(completionTask != null); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 8645d7a46c..07e1088acb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -980,6 +980,22 @@ public void ResetCommandTimeout() protected override DbParameter CreateDbParameter() => CreateParameter(); + /// + protected override void Dispose(bool disposing) + { + if (disposing) + { + // Release managed objects + _cachedMetaData = null; + + // Reset async cache information to allow a second async execute + CachedAsyncState?.ResetAsyncState(); + } + + // Release unmanaged objects + base.Dispose(disposing); + } + #endregion #region Private Methods From 9a3c9854d3edc08b0d4d1236437b9068b1a7269f Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:42:06 -0500 Subject: [PATCH 08/16] Merge RegisterColumnEncryptionKeyStoreProvidersOnCommand --- .../Data/SqlClient/SqlCommand.netcore.cs | 14 -------------- .../Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 14 -------------- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 15 +++++++++++++++ 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 6388172c8d..104864cba5 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -562,20 +562,6 @@ private EnclaveSessionParameters GetEnclaveSessionParameters() this._activeConnection.Database); } - /// - public void RegisterColumnEncryptionKeyStoreProvidersOnCommand(IDictionary customProviders) - { - ValidateCustomProviders(customProviders); - - // Create a temporary dictionary and then add items from the provided dictionary. - // Dictionary constructor does shallow copying by simply copying the provider name and provider reference pairs - // in the provided customerProviders dictionary. - Dictionary customColumnEncryptionKeyStoreProviders = - new(customProviders, StringComparer.OrdinalIgnoreCase); - - _customColumnEncryptionKeyStoreProviders = customColumnEncryptionKeyStoreProviders; - } - private void ValidateCustomProviders(IDictionary customProviders) { // Throw when the provided dictionary is null. diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index e21cb94795..4d4b5587b4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -540,20 +540,6 @@ private EnclaveSessionParameters GetEnclaveSessionParameters() this._activeConnection.Database); } - /// - public void RegisterColumnEncryptionKeyStoreProvidersOnCommand(IDictionary customProviders) - { - ValidateCustomProviders(customProviders); - - // Create a temporary dictionary and then add items from the provided dictionary. - // Dictionary constructor does shallow copying by simply copying the provider name and provider reference pairs - // in the provided customerProviders dictionary. - Dictionary customColumnEncryptionKeyStoreProviders = - new(customProviders, StringComparer.OrdinalIgnoreCase); - - _customColumnEncryptionKeyStoreProviders = customColumnEncryptionKeyStoreProviders; - } - private void ValidateCustomProviders(IDictionary customProviders) { // Throw when the provided dictionary is null. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 07e1088acb..89739110a1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.ComponentModel; using System.Data; using System.Data.Common; @@ -959,6 +960,20 @@ public override void Prepare() } } + /// + public void RegisterColumnEncryptionKeyStoreProvidersOnCommand( + IDictionary customProviders) + { + ValidateCustomProviders(customProviders); + + // Create a temporary dictionary and then add items from the provided dictionary. + // Dictionary constructor does shallow copying by simply copying the provider name and + // provider name and provider reference pairs. + Dictionary customColumnEncryptionKeyStoreProviders = + new(customProviders, StringComparer.OrdinalIgnoreCase); + _customColumnEncryptionKeyStoreProviders = customColumnEncryptionKeyStoreProviders; + } + #if NET // @TODO: Why not just expose this for netfx? /// From 507f13fd936136c44b198547cc26815aa60b292e Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:54:01 -0500 Subject: [PATCH 09/16] Remove dead method GetSqlParameterWithQueryText --- .../Microsoft/Data/SqlClient/SqlCommand.netcore.cs | 13 ------------- .../Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 13 ------------- 2 files changed, 26 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 104864cba5..2e7b866f7b 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -1604,19 +1604,6 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo( } } - /// - /// Constructs a SqlParameter with a given string value - /// - /// - /// - private SqlParameter GetSqlParameterWithQueryText(string queryText) - { - SqlParameter sqlParam = new SqlParameter(null, ((queryText.Length << 1) <= TdsEnums.TYPE_SIZE_LIMIT) ? SqlDbType.NVarChar : SqlDbType.NText, queryText.Length); - sqlParam.Value = queryText; - - return sqlParam; - } - /// /// Constructs the sp_describe_parameter_encryption request with the values from the original RPC call. /// Prototype for <sp_describe_parameter_encryption> is diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index 4d4b5587b4..ff7c796704 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -1575,19 +1575,6 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo( } } - /// - /// Constructs a SqlParameter with a given string value - /// - /// - /// - private SqlParameter GetSqlParameterWithQueryText(string queryText) - { - SqlParameter sqlParam = new SqlParameter(null, ((queryText.Length << 1) <= TdsEnums.TYPE_SIZE_LIMIT) ? SqlDbType.NVarChar : SqlDbType.NText, queryText.Length); - sqlParam.Value = queryText; - - return sqlParam; - } - /// /// Constructs the sp_describe_parameter_encryption request with the values from the original RPC call. /// Prototype for <sp_describe_parameter_encryption> is From 98a200f9e21ef5a8e3718f104244e5f3eff3e1c3 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:54:25 -0500 Subject: [PATCH 10/16] Merge OnDoneDescribeParameterEncryptionProc --- .../Data/SqlClient/SqlCommand.netcore.cs | 10 ---------- .../Data/SqlClient/SqlCommand.netfx.cs | 10 ---------- .../Microsoft/Data/SqlClient/SqlCommand.cs | 20 +++++++++++++++++++ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 2e7b866f7b..a6b4633e83 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -2232,16 +2232,6 @@ private void PutStateObject() } } - internal void OnDoneDescribeParameterEncryptionProc(TdsParserStateObject stateObj) - { - // called per rpc batch complete - if (_batchRPCMode) - { - OnDone(stateObj, _currentlyExecutingDescribeParameterEncryptionRPC, _sqlRPCParameterEncryptionReqArray, _rowsAffected); - _currentlyExecutingDescribeParameterEncryptionRPC++; - } - } - internal void OnDoneProc(TdsParserStateObject stateObject) { // called per rpc batch complete diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index ff7c796704..b27ce60f4d 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -2206,16 +2206,6 @@ private void PutStateObject() } } - internal void OnDoneDescribeParameterEncryptionProc(TdsParserStateObject stateObj) - { - // called per rpc batch complete - if (_batchRPCMode) - { - OnDone(stateObj, _currentlyExecutingDescribeParameterEncryptionRPC, _sqlRPCParameterEncryptionReqArray, _rowsAffected); - _currentlyExecutingDescribeParameterEncryptionRPC++; - } - } - internal void OnDoneProc(TdsParserStateObject stateObject) { // called per rpc batch complete diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 89739110a1..503b070f2f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -989,6 +989,26 @@ public void ResetCommandTimeout() #endregion + #region Internal Methods + + internal void OnDoneDescribeParameterEncryptionProc(TdsParserStateObject stateObj) + { + // @TODO: Is this not the same stateObj as the currently stored one? + + // Called on RPC batch complete + if (_batchRPCMode) + { + OnDone( + stateObj, + index: _currentlyExecutingDescribeParameterEncryptionRPC, + rpcList: _sqlRPCParameterEncryptionReqArray, + _rowsAffected); + _currentlyExecutingDescribeParameterEncryptionRPC++; // @TODO: Should be interlocked? + } + } + + #endregion + #region Protected Methods /// From 5480ab09de2abcb8d11c7a267a976db4843a9846 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 12:56:43 -0500 Subject: [PATCH 11/16] Merge OnDoneProc --- .../Microsoft/Data/SqlClient/SqlCommand.netcore.cs | 11 ----------- .../Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 11 ----------- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 14 ++++++++++++++ 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index a6b4633e83..bb401bc6c9 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -2232,17 +2232,6 @@ private void PutStateObject() } } - internal void OnDoneProc(TdsParserStateObject stateObject) - { - // called per rpc batch complete - if (_batchRPCMode) - { - OnDone(stateObject, _currentlyExecutingBatch, _RPCList, _rowsAffected); - _currentlyExecutingBatch++; - Debug.Assert(_RPCList.Count >= _currentlyExecutingBatch, "OnDoneProc: Too many DONEPROC events"); - } - } - private static void OnDone(TdsParserStateObject stateObj, int index, IList<_SqlRPC> rpcList, int rowsAffected) { _SqlRPC current = rpcList[index]; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index b27ce60f4d..8ecba90b77 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -2206,17 +2206,6 @@ private void PutStateObject() } } - internal void OnDoneProc(TdsParserStateObject stateObject) - { - // called per rpc batch complete - if (_batchRPCMode) - { - OnDone(stateObject, _currentlyExecutingBatch, _RPCList, _rowsAffected); - _currentlyExecutingBatch++; - Debug.Assert(_RPCList.Count >= _currentlyExecutingBatch, "OnDoneProc: Too many DONEPROC events"); - } - } - private static void OnDone(TdsParserStateObject stateObj, int index, IList<_SqlRPC> rpcList, int rowsAffected) { _SqlRPC current = rpcList[index]; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 503b070f2f..091d0ba0a6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1007,6 +1007,20 @@ internal void OnDoneDescribeParameterEncryptionProc(TdsParserStateObject stateOb } } + internal void OnDoneProc(TdsParserStateObject stateObject) + { + // @TODO: Is this not the same stateObj as the currently stored one? + + // Called on RPC batch complete + if (_batchRPCMode) + { + OnDone(stateObject, _currentlyExecutingBatch, _RPCList, _rowsAffected); + _currentlyExecutingBatch++; // @TODO: Should be interlocked? + + Debug.Assert(_RPCList.Count >= _currentlyExecutingBatch, "OnDoneProc: Too many DONEPROC events"); + } + } + #endregion #region Protected Methods From 3d7e85c509f86143918b336c409c3345c2faffc1 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 13:07:35 -0500 Subject: [PATCH 12/16] Merge OnDone --- .../Data/SqlClient/SqlCommand.netcore.cs | 32 ------------------- .../Data/SqlClient/SqlCommand.netfx.cs | 32 ------------------- .../Microsoft/Data/SqlClient/SqlCommand.cs | 32 +++++++++++++++++-- 3 files changed, 30 insertions(+), 66 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index bb401bc6c9..b3c913ba83 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -2232,38 +2232,6 @@ private void PutStateObject() } } - private static void OnDone(TdsParserStateObject stateObj, int index, IList<_SqlRPC> rpcList, int rowsAffected) - { - _SqlRPC current = rpcList[index]; - _SqlRPC previous = (index > 0) ? rpcList[index - 1] : null; - - // track the records affected for the just completed rpc batch - // _rowsAffected is cumulative for ExecuteNonQuery across all rpc batches - current.cumulativeRecordsAffected = rowsAffected; - - current.recordsAffected = - (((previous != null) && (0 <= rowsAffected)) - ? (rowsAffected - Math.Max(previous.cumulativeRecordsAffected, 0)) - : rowsAffected); - - if (current.batchCommand != null) - { - current.batchCommand.SetRecordAffected(current.recordsAffected.GetValueOrDefault()); - } - - // track the error collection (not available from TdsParser after ExecuteNonQuery) - // and the which errors are associated with the just completed rpc batch - current.errorsIndexStart = previous?.errorsIndexEnd ?? 0; - current.errorsIndexEnd = stateObj.ErrorCount; - current.errors = stateObj._errors; - - // track the warning collection (not available from TdsParser after ExecuteNonQuery) - // and the which warnings are associated with the just completed rpc batch - current.warningsIndexStart = previous?.warningsIndexEnd ?? 0; - current.warningsIndexEnd = stateObj.WarningCount; - current.warnings = stateObj._warnings; - } - internal void OnReturnStatus(int status) { // Don't set the return status if this is the status for sp_describe_parameter_encryption. diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index 8ecba90b77..bc21db6594 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -2206,38 +2206,6 @@ private void PutStateObject() } } - private static void OnDone(TdsParserStateObject stateObj, int index, IList<_SqlRPC> rpcList, int rowsAffected) - { - _SqlRPC current = rpcList[index]; - _SqlRPC previous = (index > 0) ? rpcList[index - 1] : null; - - // track the records affected for the just completed rpc batch - // _rowsAffected is cumulative for ExecuteNonQuery across all rpc batches - current.cumulativeRecordsAffected = rowsAffected; - - current.recordsAffected = - (((previous != null) && (0 <= rowsAffected)) - ? (rowsAffected - Math.Max(previous.cumulativeRecordsAffected, 0)) - : rowsAffected); - - if (current.batchCommand != null) - { - current.batchCommand.SetRecordAffected(current.recordsAffected.GetValueOrDefault()); - } - - // track the error collection (not available from TdsParser after ExecuteNonQuery) - // and the which errors are associated with the just completed rpc batch - current.errorsIndexStart = previous?.errorsIndexEnd ?? 0; - current.errorsIndexEnd = stateObj.ErrorCount; - current.errors = stateObj._errors; - - // track the warning collection (not available from TdsParser after ExecuteNonQuery) - // and the which warnings are associated with the just completed rpc batch - current.warningsIndexStart = previous?.warningsIndexEnd ?? 0; - current.warningsIndexEnd = stateObj.WarningCount; - current.warnings = stateObj._warnings; - } - internal void OnReturnStatus(int status) { if (_inPrepare) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 091d0ba0a6..4ade912c1b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1049,8 +1049,36 @@ protected override void Dispose(bool disposing) #region Private Methods - // @TODO: Rename to PrepareInternal - private void InternalPrepare() + private static void OnDone(TdsParserStateObject stateObj, int index, IList<_SqlRPC> rpcList, int rowsAffected) + { + // @TODO: Is the state object not the same as the currently stored one? + + _SqlRPC current = rpcList[index]; + _SqlRPC previous = index > 0 ? rpcList[index - 1] : null; + + // Track the records affected for the just-completed RPC batch. + // _rowsAffected is cumulative for ExecuteNonQuery across all RPC batches + current.cumulativeRecordsAffected = rowsAffected; + current.recordsAffected = previous is not null && rowsAffected >= 0 + ? rowsAffected - Math.Max(previous.cumulativeRecordsAffected, 0) + : rowsAffected; + + current.batchCommand?.SetRecordAffected(current.recordsAffected.GetValueOrDefault()); + + // Track the error collection (not available from TdsParser after ExecuteNonQuery) + // and which errors are associated with the just-completed RPC batch. + current.errorsIndexStart = previous?.errorsIndexEnd ?? 0; + current.errorsIndexEnd = stateObj.ErrorCount; + current.errors = stateObj._errors; + + // Track the warning collection (not available from TdsParser after ExecuteNonQuery) + // and which warnings are associated with the just-completed RPC batch. + current.warningsIndexStart = previous?.warningsIndexEnd ?? 0; + current.warningsIndexEnd = stateObj.WarningCount; + current.warnings = stateObj._warnings; + } + + private void PrepareInternal() { if (IsDirty) { From 9d2b6fcdf50b1046fd70d16151af406d1028f86c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 4 Sep 2025 15:43:48 -0500 Subject: [PATCH 13/16] Revert rename of PrepareInternal to InternalPrepare --- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 4ade912c1b..dc3f088ce8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1078,7 +1078,8 @@ private static void OnDone(TdsParserStateObject stateObj, int index, IList<_SqlR current.warnings = stateObj._warnings; } - private void PrepareInternal() + // @TODO: Rename PrepareInternal + private void InternalPrepare() { if (IsDirty) { From 255a7876c4e82ee1dbd4f716c23c7f68743ad9e5 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 13:39:13 -0500 Subject: [PATCH 14/16] Merge OnReturnStatus --- .../Data/SqlClient/SqlCommand.netcore.cs | 54 ----------------- .../Data/SqlClient/SqlCommand.netfx.cs | 59 ------------------- .../Microsoft/Data/SqlClient/SqlCommand.cs | 59 +++++++++++++++++++ .../Data/SqlClient/SqlQueryMetadataCache.cs | 1 + 4 files changed, 60 insertions(+), 113 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index b3c913ba83..4cc608d1c4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -2232,60 +2232,6 @@ private void PutStateObject() } } - internal void OnReturnStatus(int status) - { - // Don't set the return status if this is the status for sp_describe_parameter_encryption. - if (_inPrepare || IsDescribeParameterEncryptionRPCCurrentlyInProgress) - { - return; - } - - SqlParameterCollection parameters = _parameters; - if (_batchRPCMode) - { - if (_RPCList.Count > _currentlyExecutingBatch) - { - parameters = _RPCList[_currentlyExecutingBatch].userParams; - } - else - { - Debug.Fail("OnReturnStatus: SqlCommand got too many DONEPROC events"); - parameters = null; - } - } - // see if a return value is bound - int count = GetParameterCount(parameters); - for (int i = 0; i < count; i++) - { - SqlParameter parameter = parameters[i]; - if (parameter.Direction == ParameterDirection.ReturnValue) - { - object v = parameter.Value; - - // if the user bound a sqlint32 (the only valid one for status, use it) - if (v != null && (v.GetType() == typeof(SqlInt32))) - { - parameter.Value = new SqlInt32(status); // value type - } - else - { - parameter.Value = status; - } - - // If we are not in Batch RPC mode, update the query cache with the encryption MD. - // We can do this now that we have distinguished between ReturnValue and ReturnStatus. - // Read comment in AddQueryMetadata() for more details. - if (!_batchRPCMode && CachingQueryMetadataPostponed && - ShouldCacheEncryptionMetadata && (_parameters is not null && _parameters.Count > 0)) - { - SqlQueryMetadataCache.GetInstance().AddQueryMetadata(this, ignoreQueriesWithReturnValueParams: false); - } - - break; - } - } - } - // // Move the return value to the corresponding output parameter. // Return parameters are sent in the order in which they were defined in the procedure. diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index bc21db6594..baf55f7a62 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -2206,65 +2206,6 @@ private void PutStateObject() } } - internal void OnReturnStatus(int status) - { - if (_inPrepare) - { - return; - } - - // Don't set the return status if this is the status for sp_describe_parameter_encryption. - if (IsDescribeParameterEncryptionRPCCurrentlyInProgress) - { - return; - } - - SqlParameterCollection parameters = _parameters; - if (_batchRPCMode) - { - if (_RPCList.Count > _currentlyExecutingBatch) - { - parameters = _RPCList[_currentlyExecutingBatch].userParams; - } - else - { - Debug.Fail("OnReturnStatus: SqlCommand got too many DONEPROC events"); - parameters = null; - } - } - // see if a return value is bound - int count = GetParameterCount(parameters); - for (int i = 0; i < count; i++) - { - SqlParameter parameter = parameters[i]; - if (parameter.Direction == ParameterDirection.ReturnValue) - { - object v = parameter.Value; - - // if the user bound a sqlint32 (the only valid one for status, use it) - if (v != null && (v.GetType() == typeof(SqlInt32))) - { - parameter.Value = new SqlInt32(status); // value type - } - else - { - parameter.Value = status; - } - - // If we are not in Batch RPC mode, update the query cache with the encryption MD. - // We can do this now that we have distinguished between ReturnValue and ReturnStatus. - // Read comment in AddQueryMetadata() for more details. - if (!_batchRPCMode && CachingQueryMetadataPostponed && - ShouldCacheEncryptionMetadata && (_parameters is not null && _parameters.Count > 0)) - { - SqlQueryMetadataCache.GetInstance().AddQueryMetadata(this, ignoreQueriesWithReturnValueParams: false); - } - - break; - } - } - } - // // Move the return value to the corresponding output parameter. // Return parameters are sent in the order in which they were defined in the procedure. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index dc3f088ce8..9cf0c7f49e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -7,6 +7,7 @@ using System.ComponentModel; using System.Data; using System.Data.Common; +using System.Data.SqlTypes; using System.Diagnostics; using System.Threading; using Microsoft.Data.Common; @@ -1021,6 +1022,64 @@ internal void OnDoneProc(TdsParserStateObject stateObject) } } + internal void OnReturnStatus(int status) + { + // Don't set the return status if this is the status for sp_describe_parameter_encryption + if (_inPrepare || IsDescribeParameterEncryptionRPCCurrentlyInProgress) + { + return; + } + + SqlParameterCollection parameters = _parameters; + if (_batchRPCMode) + { + if (_RPCList.Count > _currentlyExecutingBatch) + { + parameters = _RPCList[_currentlyExecutingBatch].userParams; + } + else + { + Debug.Fail("OnReturnStatus: SqlCommand got too many DONEPROC events"); + } + } + + // See if a return value is bound + int count = GetParameterCount(parameters); + for (int i = 0; i < count; i++) + { + SqlParameter parameter = parameters[i]; + + // @TODO: Invert to reduce nesting :) + if (parameter.Direction is ParameterDirection.ReturnValue) + { + object value = parameter.Value; + + // if the user bound a SqlInt32 (the only valid one for status) use it + // @TODO: Not sure if this can be converted to a ternary since that forces implicit conversion of status to SqlInt32 + if (value is SqlInt32) + { + parameter.Value = new SqlInt32(status); + } + else + { + parameter.Value = status; + } + + // If we are not in batch RPC mode, update the query cache with the encryption + // metadata. We can do this now if we have distinguished between ReturnValue + // and ReturnStatus. + // See comments in AddQueryMetadata() for more details. + if (!_batchRPCMode && CachingQueryMetadataPostponed && ShouldCacheEncryptionMetadata && + _parameters?.Count > 0) + { + SqlQueryMetadataCache.GetInstance().AddQueryMetadata( + this, + ignoreQueriesWithReturnValueParams: false); + } + } + } + } + #endregion #region Protected Methods diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs index 964e46aca3..0af5ea44e6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs @@ -37,6 +37,7 @@ private SqlQueryMetadataCache() _cache = new MemoryCache(new MemoryCacheOptions()); } + // @TODO: Replace with Instance property. internal static SqlQueryMetadataCache GetInstance() { return s_singletonInstance; From 750abd317595ddf271852bc281588107a5d8a811 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Wed, 6 Aug 2025 14:12:51 -0500 Subject: [PATCH 15/16] Merge OnReturnValue --- .../Data/SqlClient/SqlCommand.netcore.cs | 160 ---------------- .../Data/SqlClient/SqlCommand.netfx.cs | 160 ---------------- .../Microsoft/Data/SqlClient/SqlCommand.cs | 176 ++++++++++++++++++ .../Data/SqlClient/TdsParserHelperClasses.cs | 1 + 4 files changed, 177 insertions(+), 320 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs index 4cc608d1c4..7a0491ad0f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs @@ -2232,166 +2232,6 @@ private void PutStateObject() } } - // - // Move the return value to the corresponding output parameter. - // Return parameters are sent in the order in which they were defined in the procedure. - // If named, match the parameter name, otherwise fill in based on ordinal position. - // If the parameter is not bound, then ignore the return value. - // - internal void OnReturnValue(SqlReturnValue rec, TdsParserStateObject stateObj) - { - if (_inPrepare) - { - if (!rec.value.IsNull) - { - _prepareHandle = rec.value.Int32; - } - _inPrepare = false; - return; - } - - SqlParameterCollection parameters = GetCurrentParameterCollection(); - int count = GetParameterCount(parameters); - - SqlParameter thisParam = GetParameterForOutputValueExtraction(parameters, rec.parameter, count); - - if (thisParam != null) - { - // If the parameter's direction is InputOutput, Output or ReturnValue and it needs to be transparently encrypted/decrypted - // then simply decrypt, deserialize and set the value. - if (rec.cipherMD != null && - thisParam.CipherMetadata != null && - (thisParam.Direction == ParameterDirection.Output || - thisParam.Direction == ParameterDirection.InputOutput || - thisParam.Direction == ParameterDirection.ReturnValue)) - { - if (rec.tdsType != TdsEnums.SQLBIGVARBINARY) - { - throw SQL.InvalidDataTypeForEncryptedParameter(thisParam.GetPrefixedParameterName(), rec.tdsType, TdsEnums.SQLBIGVARBINARY); - } - - // Decrypt the ciphertext - TdsParser parser = _activeConnection.Parser; - if ((parser == null) || (parser.State == TdsParserState.Closed) || (parser.State == TdsParserState.Broken)) - { - throw ADP.ClosedConnectionError(); - } - - if (!rec.value.IsNull) - { - try - { - Debug.Assert(_activeConnection != null, @"_activeConnection should not be null"); - - // Get the key information from the parameter and decrypt the value. - rec.cipherMD.EncryptionInfo = thisParam.CipherMetadata.EncryptionInfo; - byte[] unencryptedBytes = SqlSecurityUtility.DecryptWithKey(rec.value.ByteArray, rec.cipherMD, _activeConnection, this); - - if (unencryptedBytes != null) - { - // Denormalize the value and convert it to the parameter type. - SqlBuffer buffer = new SqlBuffer(); - parser.DeserializeUnencryptedValue(buffer, unencryptedBytes, rec, stateObj, rec.NormalizationRuleVersion); - thisParam.SetSqlBuffer(buffer); - } - } - catch (Exception e) - { - throw SQL.ParamDecryptionFailed(thisParam.GetPrefixedParameterName(), null, e); - } - } - else - { - // Create a new SqlBuffer and set it to null - // Note: We can't reuse the SqlBuffer in "rec" below since it's already been set (to varbinary) - // in previous call to TryProcessReturnValue(). - // Note 2: We will be coming down this code path only if the Command Setting is set to use TCE. - // We pass the command setting as TCE enabled in the below call for this reason. - SqlBuffer buff = new SqlBuffer(); - TdsParser.GetNullSqlValue(buff, rec, SqlCommandColumnEncryptionSetting.Enabled, parser.Connection); - thisParam.SetSqlBuffer(buff); - } - } - else - { - // copy over data - - // if the value user has supplied a SqlType class, then just copy over the SqlType, otherwise convert - // to the com type - object val = thisParam.Value; - - //set the UDT value as typed object rather than bytes - if (SqlDbType.Udt == thisParam.SqlDbType) - { - object data = null; - try - { - Connection.CheckGetExtendedUDTInfo(rec, true); - - //extract the byte array from the param value - if (rec.value.IsNull) - { - data = DBNull.Value; - } - else - { - data = rec.value.ByteArray; //should work for both sql and non-sql values - } - - //call the connection to instantiate the UDT object - thisParam.Value = Connection.GetUdtValue(data, rec, false); - } - catch (FileNotFoundException e) - { - // Assign Assembly.Load failure in case where assembly not on client. - // This allows execution to complete and failure on SqlParameter.Value. - thisParam.SetUdtLoadError(e); - } - catch (FileLoadException e) - { - // Assign Assembly.Load failure in case where assembly cannot be loaded on client. - // This allows execution to complete and failure on SqlParameter.Value. - thisParam.SetUdtLoadError(e); - } - - return; - } - else - { - thisParam.SetSqlBuffer(rec.value); - } - - MetaType mt = MetaType.GetMetaTypeFromSqlDbType(rec.type, false); - - if (rec.type == SqlDbType.Decimal) - { - thisParam.ScaleInternal = rec.scale; - thisParam.PrecisionInternal = rec.precision; - } - else if (mt.IsVarTime) - { - thisParam.ScaleInternal = rec.scale; - } - else if (rec.type == SqlDbType.Xml) - { - SqlCachedBuffer cachedBuffer = (thisParam.Value as SqlCachedBuffer); - if (cachedBuffer != null) - { - thisParam.Value = cachedBuffer.ToString(); - } - } - - if (rec.collation != null) - { - Debug.Assert(mt.IsCharType, "Invalid collation structure for non-char type"); - thisParam.Collation = rec.collation; - } - } - } - - return; - } - private SqlParameterCollection GetCurrentParameterCollection() { if (_batchRPCMode) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index baf55f7a62..173d46eef8 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -2206,166 +2206,6 @@ private void PutStateObject() } } - // - // Move the return value to the corresponding output parameter. - // Return parameters are sent in the order in which they were defined in the procedure. - // If named, match the parameter name, otherwise fill in based on ordinal position. - // If the parameter is not bound, then ignore the return value. - // - internal void OnReturnValue(SqlReturnValue rec, TdsParserStateObject stateObj) - { - if (_inPrepare) - { - if (!rec.value.IsNull) - { - _prepareHandle = rec.value.Int32; - } - _inPrepare = false; - return; - } - - SqlParameterCollection parameters = GetCurrentParameterCollection(); - int count = GetParameterCount(parameters); - - SqlParameter thisParam = GetParameterForOutputValueExtraction(parameters, rec.parameter, count); - - if (thisParam != null) - { - // If the parameter's direction is InputOutput, Output or ReturnValue and it needs to be transparently encrypted/decrypted - // then simply decrypt, deserialize and set the value. - if (rec.cipherMD != null && - thisParam.CipherMetadata != null && - (thisParam.Direction == ParameterDirection.Output || - thisParam.Direction == ParameterDirection.InputOutput || - thisParam.Direction == ParameterDirection.ReturnValue)) - { - if (rec.tdsType != TdsEnums.SQLBIGVARBINARY) - { - throw SQL.InvalidDataTypeForEncryptedParameter(thisParam.GetPrefixedParameterName(), rec.tdsType, TdsEnums.SQLBIGVARBINARY); - } - - // Decrypt the ciphertext - TdsParser parser = _activeConnection.Parser; - if ((parser == null) || (parser.State == TdsParserState.Closed) || (parser.State == TdsParserState.Broken)) - { - throw ADP.ClosedConnectionError(); - } - - if (!rec.value.IsNull) - { - try - { - Debug.Assert(_activeConnection != null, @"_activeConnection should not be null"); - - // Get the key information from the parameter and decrypt the value. - rec.cipherMD.EncryptionInfo = thisParam.CipherMetadata.EncryptionInfo; - byte[] unencryptedBytes = SqlSecurityUtility.DecryptWithKey(rec.value.ByteArray, rec.cipherMD, _activeConnection, this); - - if (unencryptedBytes != null) - { - // Denormalize the value and convert it to the parameter type. - SqlBuffer buffer = new SqlBuffer(); - parser.DeserializeUnencryptedValue(buffer, unencryptedBytes, rec, stateObj, rec.NormalizationRuleVersion); - thisParam.SetSqlBuffer(buffer); - } - } - catch (Exception e) - { - throw SQL.ParamDecryptionFailed(thisParam.GetPrefixedParameterName(), null, e); - } - } - else - { - // Create a new SqlBuffer and set it to null - // Note: We can't reuse the SqlBuffer in "rec" below since it's already been set (to varbinary) - // in previous call to TryProcessReturnValue(). - // Note 2: We will be coming down this code path only if the Command Setting is set to use TCE. - // We pass the command setting as TCE enabled in the below call for this reason. - SqlBuffer buff = new SqlBuffer(); - TdsParser.GetNullSqlValue(buff, rec, SqlCommandColumnEncryptionSetting.Enabled, parser.Connection); - thisParam.SetSqlBuffer(buff); - } - } - else - { - // copy over data - - // if the value user has supplied a SqlType class, then just copy over the SqlType, otherwise convert - // to the com type - object val = thisParam.Value; - - //set the UDT value as typed object rather than bytes - if (SqlDbType.Udt == thisParam.SqlDbType) - { - object data = null; - try - { - Connection.CheckGetExtendedUDTInfo(rec, true); - - //extract the byte array from the param value - if (rec.value.IsNull) - { - data = DBNull.Value; - } - else - { - data = rec.value.ByteArray; //should work for both sql and non-sql values - } - - //call the connection to instantiate the UDT object - thisParam.Value = Connection.GetUdtValue(data, rec, false); - } - catch (FileNotFoundException e) - { - // Assign Assembly.Load failure in case where assembly not on client. - // This allows execution to complete and failure on SqlParameter.Value. - thisParam.SetUdtLoadError(e); - } - catch (FileLoadException e) - { - // Assign Assembly.Load failure in case where assembly cannot be loaded on client. - // This allows execution to complete and failure on SqlParameter.Value. - thisParam.SetUdtLoadError(e); - } - - return; - } - else - { - thisParam.SetSqlBuffer(rec.value); - } - - MetaType mt = MetaType.GetMetaTypeFromSqlDbType(rec.type, rec.IsMultiValued); - - if (rec.type == SqlDbType.Decimal) - { - thisParam.ScaleInternal = rec.scale; - thisParam.PrecisionInternal = rec.precision; - } - else if (mt.IsVarTime) - { - thisParam.ScaleInternal = rec.scale; - } - else if (rec.type == SqlDbType.Xml) - { - SqlCachedBuffer cachedBuffer = (thisParam.Value as SqlCachedBuffer); - if (cachedBuffer != null) - { - thisParam.Value = cachedBuffer.ToString(); - } - } - - if (rec.collation != null) - { - Debug.Assert(mt.IsCharType, "Invalid collation structure for non-char type"); - thisParam.Collation = rec.collation; - } - } - } - - return; - } - private SqlParameterCollection GetCurrentParameterCollection() { if (_batchRPCMode) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 9cf0c7f49e..7e90f7d645 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -9,6 +9,7 @@ using System.Data.Common; using System.Data.SqlTypes; using System.Diagnostics; +using System.IO; using System.Threading; using Microsoft.Data.Common; using Microsoft.Data.Sql; @@ -1030,6 +1031,7 @@ internal void OnReturnStatus(int status) return; } + // @TODO: Replace with call to GetCurrentParameterCollection SqlParameterCollection parameters = _parameters; if (_batchRPCMode) { @@ -1080,6 +1082,180 @@ internal void OnReturnStatus(int status) } } + internal void OnReturnValue(SqlReturnValue returnValue, TdsParserStateObject stateObj) + { + // Move the return value to the corresponding output parameter. + // Return parameters are sent in the order in which they were defined in the procedure. + // If named, match the parameter name, otherwise fill in based on ordinal position. If + // the parameter is not bound, then ignore the return value. + + // @TODO: Is stateObj supposed to be different than the currently stored state object? + + if (_inPrepare) + { + // Store the returned prepare handle if we are returning from sp_prepare + if (!returnValue.value.IsNull) + { + _prepareHandle = returnValue.value.Int32; + } + + _inPrepare = false; + return; + } + + SqlParameterCollection parameters = GetCurrentParameterCollection(); + int count = GetParameterCount(parameters); + + // @TODO: Rename to "ReturnParam" + SqlParameter thisParam = GetParameterForOutputValueExtraction(parameters, returnValue.parameter, count); + if (thisParam is not null) + { + // @TODO: Invert to reduce nesting :) + + // If the parameter's direction is InputOutput, Output, or ReturnValue and it needs + // to be transparently encrypted/decrypted, then simply decrypt, deserialize, and + // set the value. + if (returnValue.cipherMD is not null && + thisParam.CipherMetadata is not null && + (thisParam.Direction == ParameterDirection.Output || + thisParam.Direction == ParameterDirection.InputOutput || + thisParam.Direction == ParameterDirection.ReturnValue)) + { + // @TODO: make this a separate method + // Validate type of the return value is valid for encryption + if (returnValue.tdsType != TdsEnums.SQLBIGVARBINARY) + { + throw SQL.InvalidDataTypeForEncryptedParameter( + thisParam.GetPrefixedParameterName(), + returnValue.tdsType, + expectedDataType: TdsEnums.SQLBIGVARBINARY); + } + + // Decrypt the cipher text + TdsParser parser = _activeConnection.Parser; + if (parser is null || parser.State is TdsParserState.Closed or TdsParserState.Broken) + { + throw ADP.ClosedConnectionError(); + } + + if (!returnValue.value.IsNull) + { + try + { + Debug.Assert(_activeConnection is not null, @"_activeConnection should not be null"); + + // Get the key information from the parameter and decrypt the value. + returnValue.cipherMD.EncryptionInfo = thisParam.CipherMetadata.EncryptionInfo; + byte[] unencryptedBytes = SqlSecurityUtility.DecryptWithKey( + returnValue.value.ByteArray, + returnValue.cipherMD, + _activeConnection, + this); + + if (unencryptedBytes is not null) + { + // Denormalize the value and convert it to the parameter type. + SqlBuffer buffer = new SqlBuffer(); + parser.DeserializeUnencryptedValue( + buffer, + unencryptedBytes, + returnValue, + stateObj, + returnValue.NormalizationRuleVersion); + thisParam.SetSqlBuffer(buffer); + } + } + catch (Exception e) + { + throw SQL.ParamDecryptionFailed( + thisParam.GetPrefixedParameterName(), + serverName: null, + e); + } + } + else + { + // Create a new SqlBuffer and set it to null + // Note: We can't reuse the SqlBuffer in "returnValue" below since it's + // already been set (to varbinary) in previous call to + // TryProcessReturnValue(). + // Note 2: We will be coming down this code path only if the Command + // Setting is set to use TCE. We pass the command setting as TCE enabled in + // the below call for this reason. + SqlBuffer buff = new SqlBuffer(); + // @TODO: uhhh what? can't we just, idk, set it null in the buffer? + TdsParser.GetNullSqlValue( + buff, + returnValue, + SqlCommandColumnEncryptionSetting.Enabled, + parser.Connection); + thisParam.SetSqlBuffer(buff); + } + } + else + { + // @TODO: This should be a separate method, too + // Copy over the data + + // If the value user has supplied a SqlType class, then just copy over the + // SqlType, otherwise convert to the com type. + if (thisParam.SqlDbType is SqlDbType.Udt) + { + try + { + _activeConnection.CheckGetExtendedUDTInfo(returnValue, fThrow: true); + + // Extract the byte array from the param value + object data = returnValue.value.IsNull + ? DBNull.Value + : returnValue.value.ByteArray; + + // Call the connection to instantiate the UDT object + thisParam.Value = _activeConnection.GetUdtValue(data, returnValue, returnDBNull: false); + } + catch (Exception e) when (e is FileNotFoundException or FileLoadException) + { + // Assign Assembly.Load failure in case where assembly not on client. + // This allows execution to complete and failure on SqlParameter.Value. + thisParam.SetUdtLoadError(e); + } + + return; + } + else + { + thisParam.SetSqlBuffer(returnValue.value); + } + + // @TODO: This seems fishy to me, it seems like it should be part of the SqlReturnValue class + MetaType mt = MetaType.GetMetaTypeFromSqlDbType(returnValue.type, isMultiValued: false); + + if (returnValue.type is SqlDbType.Decimal) + { + thisParam.ScaleInternal = returnValue.scale; + thisParam.PrecisionInternal = returnValue.precision; + } + else if (mt.IsVarTime) + { + thisParam.ScaleInternal = returnValue.scale; + } + else if (returnValue.type is SqlDbType.Xml) + { + if (thisParam.Value is SqlCachedBuffer cachedBuffer) + { + thisParam.Value = cachedBuffer.ToString(); + } + } + + if (returnValue.collation is not null) + { + Debug.Assert(mt.IsCharType, "Invalid collation structure for non-char type"); + thisParam.Collation = returnValue.collation; + } + } + } + } + #endregion #region Protected Methods diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs index abceeb05b7..8f0bb915c0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs @@ -656,6 +656,7 @@ internal SqlParameter GetParameterByIndex(int index, out byte options) internal sealed class SqlReturnValue : SqlMetaDataPriv { + // @TODO: Make auto properties (and rename to match) internal string parameter; internal readonly SqlBuffer value; #if NETFRAMEWORK From 554cbe2fa98cc3e7f161eaa4af6c521fb9aaf9d5 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Thu, 11 Sep 2025 18:08:55 -0500 Subject: [PATCH 16/16] It was always in netfx ... whoops --- .../src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs | 10 ---------- .../src/Microsoft/Data/SqlClient/SqlCommand.cs | 3 --- 2 files changed, 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs index 173d46eef8..68033e95cd 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs @@ -241,16 +241,6 @@ private AsyncState CachedAsyncState private bool IsProviderRetriable => SqlConfigurableRetryFactory.IsRetriable(RetryLogicProvider); - /// - public void ResetCommandTimeout() - { - if (ADP.DefaultCommandTimeout != CommandTimeout) - { - PropertyChanging(); - _commandTimeout = DefaultCommandTimeout; - } - } - internal void OnStatementCompleted(int recordCount) { if (0 <= recordCount) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 7e90f7d645..6fc39959cd 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -976,8 +976,6 @@ public void RegisterColumnEncryptionKeyStoreProvidersOnCommand( _customColumnEncryptionKeyStoreProviders = customColumnEncryptionKeyStoreProviders; } - #if NET - // @TODO: Why not just expose this for netfx? /// public void ResetCommandTimeout() { @@ -987,7 +985,6 @@ public void ResetCommandTimeout() _commandTimeout = DefaultCommandTimeout; } } - #endif #endregion