Skip to content

Commit

Permalink
Fix | Fix race condition issues between SinglePhaseCommit and Transac…
Browse files Browse the repository at this point in the history 
…tionEnded events (#1042)
  • Loading branch information
@cheenamalhotra
cheenamalhotra authored Apr 23, 2021
1 parent 1d4522a commit c48b6d7
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 84 deletions.
81 changes: 39 additions & 42 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,24 +324,21 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
RuntimeHelpers.PrepareConstrainedRegions();
try
{
// If the connection is doomed, we can be certain that the
// transaction will eventually be rolled back, and we shouldn't
// attempt to commit it.
if (connection.IsConnectionDoomed)
lock (connection)
{
lock (connection)
// If the connection is doomed, we can be certain that the
// transaction will eventually be rolled back or has already been aborted externally, and we shouldn't
// attempt to commit it.
if (connection.IsConnectionDoomed)
{
_active = false; // set to inactive first, doesn't matter how the rest completes, this transaction is done.
_connection = null;
}

enlistment.Aborted(SQL.ConnectionDoomed());
}
else
{
Exception commitException;
lock (connection)
enlistment.Aborted(SQL.ConnectionDoomed());
}
else
{
Exception commitException;
try
{
// Now that we've acquired the lock, make sure we still have valid state for this operation.
Expand All @@ -367,40 +364,40 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
commitException = e;
connection.DoomThisConnection();
}
}
if (commitException != null)
{
// connection.ExecuteTransaction failed with exception
if (_internalTransaction.IsCommitted)
{
// Even though we got an exception, the transaction
// was committed by the server.
enlistment.Committed();
}
else if (_internalTransaction.IsAborted)
if (commitException != null)
{
// The transaction was aborted, report that to
// SysTx.
enlistment.Aborted(commitException);
// connection.ExecuteTransaction failed with exception
if (_internalTransaction.IsCommitted)
{
// Even though we got an exception, the transaction
// was committed by the server.
enlistment.Committed();
}
else if (_internalTransaction.IsAborted)
{
// The transaction was aborted, report that to
// SysTx.
enlistment.Aborted(commitException);
}
else
{
// The transaction is still active, we cannot
// know the state of the transaction.
enlistment.InDoubt(commitException);
}

// We eat the exception. This is called on the SysTx
// thread, not the applications thread. If we don't
// eat the exception an UnhandledException will occur,
// causing the process to FailFast.
}
else

connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
if (commitException == null)
{
// The transaction is still active, we cannot
// know the state of the transaction.
enlistment.InDoubt(commitException);
// connection.ExecuteTransaction succeeded
enlistment.Committed();
}

// We eat the exception. This is called on the SysTx
// thread, not the applications thread. If we don't
// eat the exception an UnhandledException will occur,
// causing the process to FailFast.
}

connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
if (commitException == null)
{
// connection.ExecuteTransaction succeeded
enlistment.Committed();
}
}
}
Expand Down
81 changes: 39 additions & 42 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,24 +391,21 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
#else
{
#endif //DEBUG
// If the connection is doomed, we can be certain that the
// transaction will eventually be rolled back, and we shouldn't
// attempt to commit it.
if (connection.IsConnectionDoomed)
lock (connection)
{
lock (connection)
// If the connection is doomed, we can be certain that the
// transaction will eventually be rolled back or has already been aborted externally, and we shouldn't
// attempt to commit it.
if (connection.IsConnectionDoomed)
{
_active = false; // set to inactive first, doesn't matter how the rest completes, this transaction is done.
_connection = null;
}

enlistment.Aborted(SQL.ConnectionDoomed());
}
else
{
Exception commitException;
lock (connection)
enlistment.Aborted(SQL.ConnectionDoomed());
}
else
{
Exception commitException;
try
{
// Now that we've acquired the lock, make sure we still have valid state for this operation.
Expand Down Expand Up @@ -437,40 +434,40 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
ADP.TraceExceptionWithoutRethrow(e);
connection.DoomThisConnection();
}
}
if (commitException != null)
{
// connection.ExecuteTransaction failed with exception
if (_internalTransaction.IsCommitted)
{
// Even though we got an exception, the transaction
// was committed by the server.
enlistment.Committed();
}
else if (_internalTransaction.IsAborted)
if (commitException != null)
{
// The transaction was aborted, report that to
// SysTx.
enlistment.Aborted(commitException);
// connection.ExecuteTransaction failed with exception
if (_internalTransaction.IsCommitted)
{
// Even though we got an exception, the transaction
// was committed by the server.
enlistment.Committed();
}
else if (_internalTransaction.IsAborted)
{
// The transaction was aborted, report that to
// SysTx.
enlistment.Aborted(commitException);
}
else
{
// The transaction is still active, we cannot
// know the state of the transaction.
enlistment.InDoubt(commitException);
}

// We eat the exception. This is called on the SysTx
// thread, not the applications thread. If we don't
// eat the exception an UnhandledException will occur,
// causing the process to FailFast.
}
else

connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
if (commitException == null)
{
// The transaction is still active, we cannot
// know the state of the transaction.
enlistment.InDoubt(commitException);
// connection.ExecuteTransaction succeeded
enlistment.Committed();
}

// We eat the exception. This is called on the SysTx
// thread, not the applications thread. If we don't
// eat the exception an UnhandledException will occur,
// causing the process to FailFast.
}

connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
if (commitException == null)
{
// connection.ExecuteTransaction succeeded
enlistment.Committed();
}
}
}
Expand Down

0 comments on commit c48b6d7

Please sign in to comment.