Skip to content

Commit 2161490

Browse files
The crutech has to come back
1 parent dae3dd2 commit 2161490

File tree

1 file changed

+15
-7
lines changed

1 file changed

+15
-7
lines changed

src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs

+15-7
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,11 @@ public class SystemTransactionContext : ITransactionContext, IEnlistmentNotifica
187187
/// </summary>
188188
protected internal System.Transactions.Transaction EnlistedTransaction { get; }
189189
/// <inheritdoc />
190-
public bool ShouldCloseSessionOnSystemTransactionCompleted { get; set; }
190+
public bool ShouldCloseSessionOnSystemTransactionCompleted
191+
{
192+
get => _shouldCloseSessionOnSystemTransactionCompleted;
193+
set => _shouldCloseSessionOnSystemTransactionCompleted = value;
194+
}
191195
/// <inheritdoc />
192196
public bool IsInActiveTransaction { get; protected set; } = true;
193197
/// <inheritdoc />
@@ -199,6 +203,7 @@ public class SystemTransactionContext : ITransactionContext, IEnlistmentNotifica
199203
private readonly System.Transactions.Transaction _originalTransaction;
200204
private readonly ManualResetEventSlim _lock = new ManualResetEventSlim(true);
201205
private volatile bool _needCompletionLocking = true;
206+
private volatile bool _shouldCloseSessionOnSystemTransactionCompleted;
202207
private bool _preparing;
203208
// Required for not locking the completion phase itself when locking session usages from concurrent threads.
204209
private static readonly AsyncLocal<bool> _bypassLock = new AsyncLocal<bool>();
@@ -488,12 +493,15 @@ protected virtual void CompleteTransaction(bool isCommitted)
488493
// In case of a rollback due to a timeout, we may have the session disposal running concurrently
489494
// to the transaction completion in a way our current locking mechanism cannot fully protect: the
490495
// session disposal "BeginProcess" can go through the Wait before it is locked but flag the
491-
// session as processing after the transaction completion has read it as not processing. To dodge
492-
// that very unlikely case, we could consider the session as still processing initially regardless
493-
// of its actual status in case of rollback by changing below condition to
494-
// "!isCommitted || _session.IsProcessing()". That would cause a Thread.Sleep in all rollback cases.
495-
// That would reinforce the impracticality of that concurrency possibility, but with an ugly crutch.
496-
var isSessionProcessing = _session.IsProcessing();
496+
// session as processing after the transaction completion has read it as not processing. This may
497+
// then cause concurrency issues in the case the transaction context has to close the session
498+
// instead of the session disposal. To dodge that unlikely case occuring witb a legacy behavior
499+
// we advise to disable, we consider, when the legacy behavior is used, that the session is still
500+
// processing initially regardless of its actual status in case of rollback. That is a crutch,
501+
// relying on Thread.Sleep() to dodge the possible concurrency issue. If users do not want to rely
502+
// on such crutch, they should disable the commit on system transaction completion setting
503+
// (transaction.use_connection_on_system_prepare).
504+
var isSessionProcessing = !isCommitted && _useConnectionOnSystemTransactionPrepare || _session.IsProcessing();
497505
try
498506
{
499507
// Allow transaction completed actions to run while others stay blocked.

0 commit comments

Comments
 (0)