Skip to content

Commit 4532c96

Browse files
cheenamalhotracheenamalhotra
authored
[Release 2.1] Fix | Fix race condition issues between SinglePhaseCommit and TransactionEnded events (#1042) (#1049)
1 parent 4957501 commit 4532c96

File tree

2 files changed

+78
-84
lines changed

2 files changed

+78
-84
lines changed

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs‎

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -321,24 +321,21 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
321321
RuntimeHelpers.PrepareConstrainedRegions();
322322
try
323323
{
324-
// If the connection is doomed, we can be certain that the
325-
// transaction will eventually be rolled back, and we shouldn't
326-
// attempt to commit it.
327-
if (connection.IsConnectionDoomed)
324+
lock (connection)
328325
{
329-
lock (connection)
326+
// If the connection is doomed, we can be certain that the
327+
// transaction will eventually be rolled back or has already been aborted externally, and we shouldn't
328+
// attempt to commit it.
329+
if (connection.IsConnectionDoomed)
330330
{
331331
_active = false; // set to inactive first, doesn't matter how the rest completes, this transaction is done.
332332
_connection = null;
333-
}
334333

335-
enlistment.Aborted(SQL.ConnectionDoomed());
336-
}
337-
else
338-
{
339-
Exception commitException;
340-
lock (connection)
334+
enlistment.Aborted(SQL.ConnectionDoomed());
335+
}
336+
else
341337
{
338+
Exception commitException;
342339
try
343340
{
344341
// Now that we've acquired the lock, make sure we still have valid state for this operation.
@@ -364,40 +361,40 @@ public void SinglePhaseCommit(SinglePhaseEnlistment enlistment)
364361
commitException = e;
365362
connection.DoomThisConnection();
366363
}
367-
}
368-
if (commitException != null)
369-
{
370-
// connection.ExecuteTransaction failed with exception
371-
if (_internalTransaction.IsCommitted)
372-
{
373-
// Even though we got an exception, the transaction
374-
// was committed by the server.
375-
enlistment.Committed();
376-
}
377-
else if (_internalTransaction.IsAborted)
364+
if (commitException != null)
378365
{
379-
// The transaction was aborted, report that to
380-
// SysTx.
381-
enlistment.Aborted(commitException);
366+
// connection.ExecuteTransaction failed with exception
367+
if (_internalTransaction.IsCommitted)
368+
{
369+
// Even though we got an exception, the transaction
370+
// was committed by the server.
371+
enlistment.Committed();
372+
}
373+
else if (_internalTransaction.IsAborted)
374+
{
375+
// The transaction was aborted, report that to
376+
// SysTx.
377+
enlistment.Aborted(commitException);
378+
}
379+
else
380+
{
381+
// The transaction is still active, we cannot
382+
// know the state of the transaction.
383+
enlistment.InDoubt(commitException);
384+
}
385+
386+
// We eat the exception. This is called on the SysTx
387+
// thread, not the applications thread. If we don't
388+
// eat the exception an UnhandledException will occur,
389+
// causing the process to FailFast.
382390
}
383-
else
391+
392+
connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
393+
if (commitException == null)
384394
{
385-
// The transaction is still active, we cannot
386-
// know the state of the transaction.
387-
enlistment.InDoubt(commitException);
395+
// connection.ExecuteTransaction succeeded
396+
enlistment.Committed();
388397
}
389-
390-
// We eat the exception. This is called on the SysTx
391-
// thread, not the applications thread. If we don't
392-
// eat the exception an UnhandledException will occur,
393-
// causing the process to FailFast.
394-
}
395-
396-
connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
397-
if (commitException == null)
398-
{
399-
// connection.ExecuteTransaction succeeded
400-
enlistment.Committed();
401398
}
402399
}
403400
}

‎src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs‎

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -391,24 +391,21 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
391391
#else
392392
{
393393
#endif //DEBUG
394-
// If the connection is doomed, we can be certain that the
395-
// transaction will eventually be rolled back, and we shouldn't
396-
// attempt to commit it.
397-
if (connection.IsConnectionDoomed)
394+
lock (connection)
398395
{
399-
lock (connection)
396+
// If the connection is doomed, we can be certain that the
397+
// transaction will eventually be rolled back or has already been aborted externally, and we shouldn't
398+
// attempt to commit it.
399+
if (connection.IsConnectionDoomed)
400400
{
401401
_active = false; // set to inactive first, doesn't matter how the rest completes, this transaction is done.
402402
_connection = null;
403-
}
404403

405-
enlistment.Aborted(SQL.ConnectionDoomed());
406-
}
407-
else
408-
{
409-
Exception commitException;
410-
lock (connection)
404+
enlistment.Aborted(SQL.ConnectionDoomed());
405+
}
406+
else
411407
{
408+
Exception commitException;
412409
try
413410
{
414411
// Now that we've acquired the lock, make sure we still have valid state for this operation.
@@ -437,40 +434,40 @@ public void SinglePhaseCommit(SysTx.SinglePhaseEnlistment enlistment)
437434
ADP.TraceExceptionWithoutRethrow(e);
438435
connection.DoomThisConnection();
439436
}
440-
}
441-
if (commitException != null)
442-
{
443-
// connection.ExecuteTransaction failed with exception
444-
if (_internalTransaction.IsCommitted)
445-
{
446-
// Even though we got an exception, the transaction
447-
// was committed by the server.
448-
enlistment.Committed();
449-
}
450-
else if (_internalTransaction.IsAborted)
437+
if (commitException != null)
451438
{
452-
// The transaction was aborted, report that to
453-
// SysTx.
454-
enlistment.Aborted(commitException);
439+
// connection.ExecuteTransaction failed with exception
440+
if (_internalTransaction.IsCommitted)
441+
{
442+
// Even though we got an exception, the transaction
443+
// was committed by the server.
444+
enlistment.Committed();
445+
}
446+
else if (_internalTransaction.IsAborted)
447+
{
448+
// The transaction was aborted, report that to
449+
// SysTx.
450+
enlistment.Aborted(commitException);
451+
}
452+
else
453+
{
454+
// The transaction is still active, we cannot
455+
// know the state of the transaction.
456+
enlistment.InDoubt(commitException);
457+
}
458+
459+
// We eat the exception. This is called on the SysTx
460+
// thread, not the applications thread. If we don't
461+
// eat the exception an UnhandledException will occur,
462+
// causing the process to FailFast.
455463
}
456-
else
464+
465+
connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
466+
if (commitException == null)
457467
{
458-
// The transaction is still active, we cannot
459-
// know the state of the transaction.
460-
enlistment.InDoubt(commitException);
468+
// connection.ExecuteTransaction succeeded
469+
enlistment.Committed();
461470
}
462-
463-
// We eat the exception. This is called on the SysTx
464-
// thread, not the applications thread. If we don't
465-
// eat the exception an UnhandledException will occur,
466-
// causing the process to FailFast.
467-
}
468-
469-
connection.CleanupConnectionOnTransactionCompletion(_atomicTransaction);
470-
if (commitException == null)
471-
{
472-
// connection.ExecuteTransaction succeeded
473-
enlistment.Committed();
474471
}
475472
}
476473
}

0 commit comments

Comments
 (0)