Skip to content

Commit 3b7fa03

Browse files
fredericDelaportefredericDelaporte
committed
Fix dependent transaction failure
Fix #2172
1 parent d636a40 commit 3b7fa03

File tree

8 files changed

+181
-20
lines changed

8 files changed

+181
-20
lines changed

‎src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313
using System.Threading;
1414
using System.Transactions;
1515
using log4net;
16-
using log4net.Repository.Hierarchy;
1716
using NHibernate.Cfg;
1817
using NHibernate.Engine;
19-
using NHibernate.Linq;
2018
using NHibernate.Test.TransactionTest;
2119
using NUnit.Framework;
2220

21+
using sysTran = System.Transactions;
22+
using NHibernate.Linq;
23+
2324
namespace NHibernate.Test.SystemTransactions
2425
{
2526
using System.Threading.Tasks;
@@ -48,7 +49,7 @@ public async Task SupportsEnlistingInDistributedAsync()
4849

4950
Assert.AreNotEqual(
5051
Guid.Empty,
51-
System.Transactions.Transaction.Current.TransactionInformation.DistributedIdentifier,
52+
sysTran.Transaction.Current.TransactionInformation.DistributedIdentifier,
5253
"Transaction lacks a distributed identifier");
5354

5455
using (var s = OpenSession())
@@ -75,7 +76,7 @@ public async Task SupportsPromotingToDistributedAsync()
7576
"Failure promoting the transaction to distributed while already having enlisted a connection.");
7677
Assert.AreNotEqual(
7778
Guid.Empty,
78-
System.Transactions.Transaction.Current.TransactionInformation.DistributedIdentifier,
79+
sysTran.Transaction.Current.TransactionInformation.DistributedIdentifier,
7980
"Transaction lacks a distributed identifier");
8081
}
8182
}
@@ -688,6 +689,39 @@ public async Task EnforceConnectionUsageRulesOnTransactionCompletionAsync()
688689
Assert.That(interceptor.AfterException, Is.TypeOf<HibernateException>());
689690
}
690691

692+
[Theory]
693+
public async Task CanUseDependentTransactionAsync(bool explicitFlush)
694+
{
695+
if (!TestDialect.SupportsDependentTransaction)
696+
Assert.Ignore("Dialect does not support dependent transactions");
697+
IgnoreIfUnsupported(explicitFlush);
698+
699+
using (var committable = new CommittableTransaction())
700+
{
701+
sysTran.Transaction.Current = committable;
702+
using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
703+
{
704+
sysTran.Transaction.Current = clone;
705+
706+
using (var s = OpenSession())
707+
{
708+
if (!AutoJoinTransaction)
709+
s.JoinTransaction();
710+
await (s.SaveAsync(new Person()));
711+
712+
if (explicitFlush)
713+
await (s.FlushAsync());
714+
clone.Complete();
715+
}
716+
}
717+
718+
sysTran.Transaction.Current = committable;
719+
committable.Commit();
720+
}
721+
722+
sysTran.Transaction.Current = null;
723+
}
724+
691725
private Task DodgeTransactionCompletionDelayIfRequiredAsync(CancellationToken cancellationToken = default(CancellationToken))
692726
{
693727
try
@@ -716,7 +750,7 @@ public class ForceEscalationToDistributedTx : IEnlistmentNotification
716750
public static void Escalate(bool shouldRollBack = false)
717751
{
718752
var force = new ForceEscalationToDistributedTx(shouldRollBack);
719-
System.Transactions.Transaction.Current.EnlistDurable(Guid.NewGuid(), force, EnlistmentOptions.None);
753+
sysTran.Transaction.Current.EnlistDurable(Guid.NewGuid(), force, EnlistmentOptions.None);
720754
}
721755

722756
private ForceEscalationToDistributedTx(bool shouldRollBack)

‎src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616
using NHibernate.Cfg;
1717
using NHibernate.Driver;
1818
using NHibernate.Engine;
19-
using NHibernate.Linq;
2019
using NHibernate.Test.TransactionTest;
2120
using NUnit.Framework;
21+
using sysTran = System.Transactions;
22+
using NHibernate.Linq;
2223

2324
namespace NHibernate.Test.SystemTransactions
2425
{
@@ -523,6 +524,44 @@ public async Task EnforceConnectionUsageRulesOnTransactionCompletionAsync()
523524
// Currently always forbidden, whatever UseConnectionOnSystemTransactionEvents.
524525
Assert.That(interceptor.AfterException, Is.TypeOf<HibernateException>());
525526
}
527+
528+
[Theory]
529+
public async Task CanUseDependentTransactionAsync(bool explicitFlush)
530+
{
531+
if (!TestDialect.SupportsDependentTransaction)
532+
Assert.Ignore("Dialect does not support dependent transactions");
533+
IgnoreIfUnsupported(explicitFlush);
534+
535+
try
536+
{
537+
using (var committable = new CommittableTransaction())
538+
{
539+
sysTran.Transaction.Current = committable;
540+
using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
541+
{
542+
sysTran.Transaction.Current = clone;
543+
544+
using (var s = OpenSession())
545+
{
546+
if (!AutoJoinTransaction)
547+
s.JoinTransaction();
548+
await (s.SaveAsync(new Person()));
549+
550+
if (explicitFlush)
551+
await (s.FlushAsync());
552+
clone.Complete();
553+
}
554+
}
555+
556+
sysTran.Transaction.Current = committable;
557+
committable.Commit();
558+
}
559+
}
560+
finally
561+
{
562+
sysTran.Transaction.Current = null;
563+
}
564+
}
526565
}
527566

528567
[TestFixture]

‎src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
using System.Threading;
44
using System.Transactions;
55
using log4net;
6-
using log4net.Repository.Hierarchy;
76
using NHibernate.Cfg;
87
using NHibernate.Engine;
9-
using NHibernate.Linq;
108
using NHibernate.Test.TransactionTest;
119
using NUnit.Framework;
1210

11+
using sysTran = System.Transactions;
12+
1313
namespace NHibernate.Test.SystemTransactions
1414
{
1515
[TestFixture]
@@ -37,7 +37,7 @@ public void SupportsEnlistingInDistributed()
3737

3838
Assert.AreNotEqual(
3939
Guid.Empty,
40-
System.Transactions.Transaction.Current.TransactionInformation.DistributedIdentifier,
40+
sysTran.Transaction.Current.TransactionInformation.DistributedIdentifier,
4141
"Transaction lacks a distributed identifier");
4242

4343
using (var s = OpenSession())
@@ -64,7 +64,7 @@ public void SupportsPromotingToDistributed()
6464
"Failure promoting the transaction to distributed while already having enlisted a connection.");
6565
Assert.AreNotEqual(
6666
Guid.Empty,
67-
System.Transactions.Transaction.Current.TransactionInformation.DistributedIdentifier,
67+
sysTran.Transaction.Current.TransactionInformation.DistributedIdentifier,
6868
"Transaction lacks a distributed identifier");
6969
}
7070
}
@@ -711,6 +711,39 @@ public void AdditionalJoinDoesNotThrow()
711711
}
712712
}
713713

714+
[Theory]
715+
public void CanUseDependentTransaction(bool explicitFlush)
716+
{
717+
if (!TestDialect.SupportsDependentTransaction)
718+
Assert.Ignore("Dialect does not support dependent transactions");
719+
IgnoreIfUnsupported(explicitFlush);
720+
721+
using (var committable = new CommittableTransaction())
722+
{
723+
sysTran.Transaction.Current = committable;
724+
using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
725+
{
726+
sysTran.Transaction.Current = clone;
727+
728+
using (var s = OpenSession())
729+
{
730+
if (!AutoJoinTransaction)
731+
s.JoinTransaction();
732+
s.Save(new Person());
733+
734+
if (explicitFlush)
735+
s.Flush();
736+
clone.Complete();
737+
}
738+
}
739+
740+
sysTran.Transaction.Current = committable;
741+
committable.Commit();
742+
}
743+
744+
sysTran.Transaction.Current = null;
745+
}
746+
714747
private void DodgeTransactionCompletionDelayIfRequired()
715748
{
716749
if (Sfi.ConnectionProvider.Driver.HasDelayedDistributedTransactionCompletion)
@@ -725,7 +758,7 @@ public class ForceEscalationToDistributedTx : IEnlistmentNotification
725758
public static void Escalate(bool shouldRollBack = false)
726759
{
727760
var force = new ForceEscalationToDistributedTx(shouldRollBack);
728-
System.Transactions.Transaction.Current.EnlistDurable(Guid.NewGuid(), force, EnlistmentOptions.None);
761+
sysTran.Transaction.Current.EnlistDurable(Guid.NewGuid(), force, EnlistmentOptions.None);
729762
}
730763

731764
private ForceEscalationToDistributedTx(bool shouldRollBack)

‎src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
using NHibernate.Cfg;
77
using NHibernate.Driver;
88
using NHibernate.Engine;
9-
using NHibernate.Linq;
109
using NHibernate.Test.TransactionTest;
1110
using NUnit.Framework;
11+
using sysTran = System.Transactions;
1212

1313
namespace NHibernate.Test.SystemTransactions
1414
{
@@ -522,6 +522,44 @@ public void AdditionalJoinDoesNotThrow()
522522
Assert.DoesNotThrow(() => s.JoinTransaction());
523523
}
524524
}
525+
526+
[Theory]
527+
public void CanUseDependentTransaction(bool explicitFlush)
528+
{
529+
if (!TestDialect.SupportsDependentTransaction)
530+
Assert.Ignore("Dialect does not support dependent transactions");
531+
IgnoreIfUnsupported(explicitFlush);
532+
533+
try
534+
{
535+
using (var committable = new CommittableTransaction())
536+
{
537+
sysTran.Transaction.Current = committable;
538+
using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete))
539+
{
540+
sysTran.Transaction.Current = clone;
541+
542+
using (var s = OpenSession())
543+
{
544+
if (!AutoJoinTransaction)
545+
s.JoinTransaction();
546+
s.Save(new Person());
547+
548+
if (explicitFlush)
549+
s.Flush();
550+
clone.Complete();
551+
}
552+
}
553+
554+
sysTran.Transaction.Current = committable;
555+
committable.Commit();
556+
}
557+
}
558+
finally
559+
{
560+
sysTran.Transaction.Current = null;
561+
}
562+
}
525563
}
526564

527565
[TestFixture]

‎src/NHibernate.Test/TestDialect.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,14 @@ public bool SupportsSqlType(SqlType sqlType)
157157
/// </summary>
158158
public virtual bool SupportsUsingConnectionOnSystemTransactionPrepare => true;
159159

160+
/// <summary>
161+
/// Some databases fail with dependent transaction, typically when their driver try to access the transaction
162+
/// state from its two PC: the dependent transaction is meant to be disposed of before completing the actual
163+
/// transaction, so it is usually disposed at this point, and its state cannot be read. (Drivers should always
164+
/// clone transactions for avoiding this trouble.)
165+
/// </summary>
166+
public virtual bool SupportsDependentTransaction => true;
167+
160168
/// <summary>
161169
/// Some databases (provider?) fails to compute adequate column types for queries which columns
162170
/// computing include a parameter value.

‎src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,11 @@ public PostgreSQL83TestDialect(Dialect.Dialect dialect)
1616
/// Npgsql 3.2.4.1.
1717
/// </summary>
1818
public override bool SupportsUsingConnectionOnSystemTransactionPrepare => false;
19+
20+
/// <summary>
21+
/// Npgsql does not clone the transaction in its context, and use it in its prepare phase. When that was a
22+
/// dependent transaction, it is then usually already disposed of, causing Npgsql to crash.
23+
/// </summary>
24+
public override bool SupportsDependentTransaction => false;
1925
}
2026
}

‎src/NHibernate/Async/Transaction/AdoNetWithSystemTransactionFactory.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
using NHibernate.AdoNet;
1717
using NHibernate.Engine;
1818
using NHibernate.Engine.Transaction;
19-
using NHibernate.Impl;
2019
using NHibernate.Util;
2120

2221
namespace NHibernate.Transaction

‎src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using NHibernate.AdoNet;
77
using NHibernate.Engine;
88
using NHibernate.Engine.Transaction;
9-
using NHibernate.Impl;
109
using NHibernate.Util;
1110

1211
namespace NHibernate.Transaction
@@ -186,6 +185,7 @@ public class SystemTransactionContext : ITransactionContext, IEnlistmentNotifica
186185
private readonly System.Transactions.Transaction _originalTransaction;
187186
private readonly ManualResetEventSlim _lock = new ManualResetEventSlim(true);
188187
private volatile bool _needCompletionLocking = true;
188+
private bool _preparing;
189189
// Required for not locking the completion phase itself when locking session usages from concurrent threads.
190190
private static readonly AsyncLocal<bool> _bypassLock = new AsyncLocal<bool>();
191191
private readonly int _systemTransactionCompletionLockTimeout;
@@ -236,7 +236,7 @@ public virtual void Wait()
236236
// Remove the block then throw.
237237
Unlock();
238238
throw new HibernateException(
239-
"Synchronization timeout for transaction completion. Either raise {Cfg.Environment.SystemTransactionCompletionLockTimeout}, or this may be a bug in NHibernate.");
239+
$"Synchronization timeout for transaction completion. Either raise {Cfg.Environment.SystemTransactionCompletionLockTimeout}, or this may be a bug in NHibernate.");
240240
}
241241
catch (HibernateException)
242242
{
@@ -285,16 +285,17 @@ protected virtual void Unlock()
285285
{
286286
// Cloned transaction is not disposed "unexpectedly", its status is accessible till context disposal.
287287
var status = EnlistedTransaction.TransactionInformation.Status;
288-
if (status != TransactionStatus.Active)
288+
if (status != TransactionStatus.Active || _preparing)
289289
return status;
290290

291-
// The clone status can be out of date when active, check the original one (which could be disposed if
292-
// the clone is out of date).
291+
// The clone status can be out of date when active and not in prepare phase, in case of rollback.
292+
// In such case the original transaction is already disposed, an trying to check its status will
293+
// trigger a dispose exception.
293294
return _originalTransaction.TransactionInformation.Status;
294295
}
295296
catch (ObjectDisposedException ode)
296297
{
297-
_logger.Warn(ode, "Enlisted transaction status was wrongly active, original transaction being already disposed. Will assume neither active nor committed.");
298+
_logger.Warn(ode, "Enlisted transaction status is maybe wrongly active, original transaction being already disposed. Will assume neither active nor committed.");
298299
return null;
299300
}
300301
}
@@ -310,6 +311,7 @@ protected virtual void Unlock()
310311
/// <param name="preparingEnlistment">The object for notifying the prepare phase outcome.</param>
311312
public virtual void Prepare(PreparingEnlistment preparingEnlistment)
312313
{
314+
_preparing = true;
313315
using (_session.BeginContext())
314316
{
315317
try
@@ -343,10 +345,12 @@ public virtual void Prepare(PreparingEnlistment preparingEnlistment)
343345
Lock();
344346

345347
_logger.Debug("Prepared for system transaction");
348+
_preparing = false;
346349
preparingEnlistment.Prepared();
347350
}
348351
catch (Exception exception)
349352
{
353+
_preparing = false;
350354
_logger.Error(exception, "System transaction prepare phase failed");
351355
try
352356
{
@@ -404,7 +408,7 @@ protected virtual void ProcessSecondPhase(Enlistment enlistment, bool? success)
404408
/// <summary>
405409
/// Handle the transaction completion. Notify <see cref="ConnectionManager"/> of the end of the
406410
/// transaction. Notify end of transaction to the session and to <see cref="ConnectionManager.DependentSessions"/>
407-
/// if any. Close sessions requiring it then cleanup transaction contextes and then <see cref="Unlock"/> blocked
411+
/// if any. Close sessions requiring it then cleanup transaction contexts and then <see cref="Unlock"/> blocked
408412
/// threads.
409413
/// </summary>
410414
/// <param name="isCommitted"><see langword="true"/> if the transaction is committed, <see langword="false"/>

0 commit comments

Comments
 (0)