diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index 8fc8df24e0..9f4d5d2e08 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -16,6 +16,7 @@ internal abstract partial class DbConnectionInternal { private static int _objectTypeCount; internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); + private TransactionCompletedEventHandler _transactionCompletedEventHandler = null; private bool _isInStasis; @@ -437,15 +438,19 @@ internal void DetachTransaction(Transaction transaction, bool isExplicitlyReleas // potentially a multi-threaded event, so lock the connection to make sure we don't enlist in a new // transaction between compare and assignment. No need to short circuit outside of lock, since failed comparisons should // be the exception, not the rule. - lock (this) + // locking on anything other than the transaction object would lead to a thread deadlock with sys.Transaction.TransactionCompleted event. + lock (transaction) { // Detach if detach-on-end behavior, or if outer connection was closed - DbConnection owner = (DbConnection)Owner; - if (isExplicitlyReleasing || UnbindOnTransactionCompletion || null == owner) + DbConnection owner = Owner; + if (isExplicitlyReleasing || UnbindOnTransactionCompletion || owner is null) { Transaction currentEnlistedTransaction = _enlistedTransaction; if (currentEnlistedTransaction != null && transaction.Equals(currentEnlistedTransaction)) { + // We need to remove the transaction completed event handler to cease listening for the transaction to end. + currentEnlistedTransaction.TransactionCompleted -= _transactionCompletedEventHandler; + EnlistedTransaction = null; if (IsTxRootWaitingForTxEnd) @@ -479,7 +484,8 @@ void TransactionCompletedEvent(object sender, TransactionEventArgs e) private void TransactionOutcomeEnlist(Transaction transaction) { - transaction.TransactionCompleted += new TransactionCompletedEventHandler(TransactionCompletedEvent); + _transactionCompletedEventHandler ??= new TransactionCompletedEventHandler(TransactionCompletedEvent); + transaction.TransactionCompleted += _transactionCompletedEventHandler; } internal void SetInStasis() diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs index 848e69aa06..3ed756eef0 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs @@ -159,17 +159,17 @@ public byte[] Promote() ValidateActiveOnConnection(connection); connection.ExecuteTransaction(SqlInternalConnection.TransactionRequest.Promote, null, System.Data.IsolationLevel.Unspecified, _internalTransaction, true); - returnValue = _connection.PromotedDTCToken; + returnValue = connection.PromotedDTCToken; // For Global Transactions, we need to set the Transaction Id since we use a Non-MSDTC Promoter type. - if (_connection.IsGlobalTransaction) + if (connection.IsGlobalTransaction) { if (SysTxForGlobalTransactions.SetDistributedTransactionIdentifier == null) { throw SQL.UnsupportedSysTxForGlobalTransactions(); } - if (!_connection.IsGlobalTransactionsEnabledForServer) + if (!connection.IsGlobalTransactionsEnabledForServer) { throw SQL.GlobalTransactionsNotEnabled(); } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs index a79eb68411..06b1e04712 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs @@ -18,11 +18,10 @@ namespace Microsoft.Data.ProviderBase using SysTx = System.Transactions; internal abstract class DbConnectionInternal - { // V1.1.3300 - - + { private static int _objectTypeCount; internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); + private SysTx.TransactionCompletedEventHandler _transactionCompletedEventHandler = null; internal static readonly StateChangeEventArgs StateChangeClosed = new StateChangeEventArgs(ConnectionState.Open, ConnectionState.Closed); internal static readonly StateChangeEventArgs StateChangeOpen = new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open); @@ -900,15 +899,18 @@ internal void DetachTransaction(SysTx.Transaction transaction, bool isExplicitly // potentially a multi-threaded event, so lock the connection to make sure we don't enlist in a new // transaction between compare and assignment. No need to short circuit outside of lock, since failed comparisons should // be the exception, not the rule. - lock (this) + // locking on anything other than the transaction object would lead to a thread deadlock with sys.Transaction.TransactionCompleted event. + lock (transaction) { // Detach if detach-on-end behavior, or if outer connection was closed - DbConnection owner = (DbConnection)Owner; - if (isExplicitlyReleasing || UnbindOnTransactionCompletion || null == owner) + DbConnection owner = Owner; + if (isExplicitlyReleasing || UnbindOnTransactionCompletion || owner is null) { SysTx.Transaction currentEnlistedTransaction = _enlistedTransaction; if (currentEnlistedTransaction != null && transaction.Equals(currentEnlistedTransaction)) { + // We need to remove the transaction completed event handler to cease listening for the transaction to end. + currentEnlistedTransaction.TransactionCompleted -= _transactionCompletedEventHandler; EnlistedTransaction = null; @@ -947,7 +949,8 @@ void TransactionCompletedEvent(object sender, SysTx.TransactionEventArgs e) [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] private void TransactionOutcomeEnlist(SysTx.Transaction transaction) { - transaction.TransactionCompleted += new SysTx.TransactionCompletedEventHandler(TransactionCompletedEvent); + _transactionCompletedEventHandler ??= new SysTx.TransactionCompletedEventHandler(TransactionCompletedEvent); + transaction.TransactionCompleted += _transactionCompletedEventHandler; } internal void SetInStasis() diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs index ebc234d480..e11ec36e79 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs @@ -192,17 +192,17 @@ public Byte[] Promote() ValidateActiveOnConnection(connection); connection.ExecuteTransaction(SqlInternalConnection.TransactionRequest.Promote, null, IsolationLevel.Unspecified, _internalTransaction, true); - returnValue = _connection.PromotedDTCToken; + returnValue = connection.PromotedDTCToken; // For Global Transactions, we need to set the Transaction Id since we use a Non-MSDTC Promoter type. - if (_connection.IsGlobalTransaction) + if (connection.IsGlobalTransaction) { if (SysTxForGlobalTransactions.SetDistributedTransactionIdentifier == null) { throw SQL.UnsupportedSysTxForGlobalTransactions(); } - if (!_connection.IsGlobalTransactionsEnabledForServer) + if (!connection.IsGlobalTransactionsEnabledForServer) { throw SQL.GlobalTransactionsNotEnabled(); }