diff --git a/build-common/NHibernate.props b/build-common/NHibernate.props index 4da93284f67..abd091a26fb 100644 --- a/build-common/NHibernate.props +++ b/build-common/NHibernate.props @@ -2,7 +2,7 @@ 5 0 - 7 + 8 $(VersionMajor).$(VersionMinor).$(VersionPatch) diff --git a/build-common/common.xml b/build-common/common.xml index 2681d0f78b1..367b55984aa 100644 --- a/build-common/common.xml +++ b/build-common/common.xml @@ -30,8 +30,8 @@ - - + + diff --git a/releasenotes.txt b/releasenotes.txt index 1745272aa3d..55c71dca7d9 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -1,4 +1,12 @@ -Build 5.0.7 +Build 5.0.8 +============================= + +Release notes - NHibernate - Version 5.0.8 + +** Bug + * #2172 Using DependentTransaction fails + +Build 5.0.7 ============================= Release notes - NHibernate - Version 5.0.7 diff --git a/src/AsyncGenerator.yml b/src/AsyncGenerator.yml index accaaeabe72..3b7f6f0e3ac 100644 --- a/src/AsyncGenerator.yml +++ b/src/AsyncGenerator.yml @@ -164,6 +164,18 @@ applyChanges: true analyzation: methodConversion: + - conversion: Ignore + name: CanUseDependentTransaction + containingTypeName: DistributedSystemTransactionFixture + - conversion: Ignore + name: CanUseSessionWithManyDependentTransaction + containingTypeName: DistributedSystemTransactionFixture + - conversion: Ignore + name: CanUseDependentTransaction + containingTypeName: SystemTransactionFixture + - conversion: Ignore + name: CanUseSessionWithManyDependentTransaction + containingTypeName: SystemTransactionFixture - conversion: Copy hasAttributeName: OneTimeSetUpAttribute - conversion: Copy diff --git a/src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs b/src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs index ca7c64e5a9d..1f0c9f1f382 100644 --- a/src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs +++ b/src/NHibernate.Test/Async/SystemTransactions/DistributedSystemTransactionFixture.cs @@ -16,9 +16,9 @@ using log4net.Repository.Hierarchy; using NHibernate.Cfg; using NHibernate.Engine; -using NHibernate.Linq; using NHibernate.Test.TransactionTest; using NUnit.Framework; +using NHibernate.Linq; namespace NHibernate.Test.SystemTransactions { @@ -780,4 +780,4 @@ protected override void Configure(Configuration configuration) protected override bool AppliesTo(ISessionFactoryImplementor factory) => base.AppliesTo(factory) && factory.ConnectionProvider.Driver.SupportsEnlistmentWhenAutoEnlistmentIsDisabled; } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs b/src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs index 937f64528f0..6fde75a7199 100644 --- a/src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs +++ b/src/NHibernate.Test/Async/SystemTransactions/SystemTransactionFixture.cs @@ -14,10 +14,11 @@ using System.Threading; using System.Transactions; using NHibernate.Cfg; +using NHibernate.Driver; using NHibernate.Engine; -using NHibernate.Linq; using NHibernate.Test.TransactionTest; using NUnit.Framework; +using NHibernate.Linq; namespace NHibernate.Test.SystemTransactions { @@ -529,4 +530,4 @@ public class SystemTransactionWithoutAutoJoinTransactionAsync : SystemTransactio { protected override bool AutoJoinTransaction => false; } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs b/src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs index b9b8e089f0e..f6751188e2d 100644 --- a/src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs +++ b/src/NHibernate.Test/SystemTransactions/DistributedSystemTransactionFixture.cs @@ -6,7 +6,6 @@ using log4net.Repository.Hierarchy; using NHibernate.Cfg; using NHibernate.Engine; -using NHibernate.Linq; using NHibernate.Test.TransactionTest; using NUnit.Framework; @@ -728,6 +727,117 @@ public void AdditionalJoinDoesNotThrow() } } + [Theory] + public void CanUseDependentTransaction(bool explicitFlush) + { + if (!TestDialect.SupportsDependentTransaction) + Assert.Ignore("Dialect does not support dependent transactions"); + IgnoreIfUnsupported(explicitFlush); + + try + { + using (var committable = new CommittableTransaction()) + { + System.Transactions.Transaction.Current = committable; + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + + using (var s = OpenSession()) + { + if (!AutoJoinTransaction) + s.JoinTransaction(); + s.Save(new Person()); + + if (explicitFlush) + s.Flush(); + clone.Complete(); + } + } + + System.Transactions.Transaction.Current = committable; + committable.Commit(); + } + } + finally + { + System.Transactions.Transaction.Current = null; + } + } + + [Theory] + public void CanUseSessionWithManyDependentTransaction(bool explicitFlush) + { + if (!TestDialect.SupportsDependentTransaction) + Assert.Ignore("Dialect does not support dependent transactions"); + IgnoreIfUnsupported(explicitFlush); + + try + { + using (var s = Sfi.WithOptions().ConnectionReleaseMode(ConnectionReleaseMode.OnClose).OpenSession()) + { + using (var committable = new CommittableTransaction()) + { + System.Transactions.Transaction.Current = committable; + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + if (!AutoJoinTransaction) + s.JoinTransaction(); + // Acquire the connection + var count = s.Query().Count(); + Assert.That(count, Is.EqualTo(0), "Unexpected initial entity count."); + clone.Complete(); + } + + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + if (!AutoJoinTransaction) + s.JoinTransaction(); + s.Save(new Person()); + + if (explicitFlush) + s.Flush(); + + clone.Complete(); + } + + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + if (!AutoJoinTransaction) + s.JoinTransaction(); + var count = s.Query().Count(); + Assert.That(count, Is.EqualTo(1), "Unexpected entity count after committed insert."); + clone.Complete(); + } + + System.Transactions.Transaction.Current = committable; + committable.Commit(); + } + } + } + finally + { + System.Transactions.Transaction.Current = null; + } + + DodgeTransactionCompletionDelayIfRequired(); + + using (var s = OpenSession()) + { + using (var tx = new TransactionScope()) + { + if (!AutoJoinTransaction) + s.JoinTransaction(); + var count = s.Query().Count(); + Assert.That(count, Is.EqualTo(1), "Unexpected entity count after global commit."); + tx.Complete(); + } + } + } + private void DodgeTransactionCompletionDelayIfRequired() { if (Sfi.ConnectionProvider.Driver.HasDelayedDistributedTransactionCompletion) @@ -820,4 +930,4 @@ public void SessionIsNotEnlisted() } } } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs b/src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs index 18e456663b2..9fd528e1f89 100644 --- a/src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs +++ b/src/NHibernate.Test/SystemTransactions/SystemTransactionFixture.cs @@ -4,8 +4,8 @@ using System.Threading; using System.Transactions; using NHibernate.Cfg; +using NHibernate.Driver; using NHibernate.Engine; -using NHibernate.Linq; using NHibernate.Test.TransactionTest; using NUnit.Framework; @@ -502,6 +502,120 @@ public void AdditionalJoinDoesNotThrow() Assert.DoesNotThrow(() => s.JoinTransaction()); } } + + [Theory] + public void CanUseDependentTransaction(bool explicitFlush) + { + if (!TestDialect.SupportsDependentTransaction) + Assert.Ignore("Dialect does not support dependent transactions"); + IgnoreIfUnsupported(explicitFlush); + + try + { + using (var committable = new CommittableTransaction()) + { + System.Transactions.Transaction.Current = committable; + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + + using (var s = OpenSession()) + { + if (!AutoJoinTransaction) + s.JoinTransaction(); + s.Save(new Person()); + + if (explicitFlush) + s.Flush(); + clone.Complete(); + } + } + + System.Transactions.Transaction.Current = committable; + committable.Commit(); + } + } + finally + { + System.Transactions.Transaction.Current = null; + } + } + + [Theory] + public void CanUseSessionWithManyDependentTransaction(bool explicitFlush) + { + if (!TestDialect.SupportsDependentTransaction) + Assert.Ignore("Dialect does not support dependent transactions"); + IgnoreIfUnsupported(explicitFlush); + // ODBC with SQL-Server always causes system transactions to go distributed, which causes their transaction completion to run + // asynchronously. But ODBC enlistment also check the previous transaction in a way that do not guard against it + // being concurrently disposed of. See https://github.com/nhibernate/nhibernate-core/pull/1505 for more details. + if (Sfi.ConnectionProvider.Driver is OdbcDriver) + Assert.Ignore("ODBC sometimes fails on second scope by checking the previous transaction status, which may yield an object disposed exception"); + + try + { + using (var s = WithOptions().ConnectionReleaseMode(ConnectionReleaseMode.OnClose).OpenSession()) + { + using (var committable = new CommittableTransaction()) + { + System.Transactions.Transaction.Current = committable; + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + if (!AutoJoinTransaction) + s.JoinTransaction(); + // Acquire the connection + var count = s.Query().Count(); + Assert.That(count, Is.EqualTo(0), "Unexpected initial entity count."); + clone.Complete(); + } + + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + if (!AutoJoinTransaction) + s.JoinTransaction(); + s.Save(new Person()); + + if (explicitFlush) + s.Flush(); + + clone.Complete(); + } + + using (var clone = committable.DependentClone(DependentCloneOption.RollbackIfNotComplete)) + { + System.Transactions.Transaction.Current = clone; + if (!AutoJoinTransaction) + s.JoinTransaction(); + var count = s.Query().Count(); + Assert.That(count, Is.EqualTo(1), "Unexpected entity count after committed insert."); + clone.Complete(); + } + + System.Transactions.Transaction.Current = committable; + committable.Commit(); + } + } + } + finally + { + System.Transactions.Transaction.Current = null; + } + + using (var s = OpenSession()) + { + using (var tx = new TransactionScope()) + { + if (!AutoJoinTransaction) + s.JoinTransaction(); + var count = s.Query().Count(); + Assert.That(count, Is.EqualTo(1), "Unexpected entity count after global commit."); + tx.Complete(); + } + } + } } [TestFixture] @@ -539,4 +653,4 @@ public void SessionIsNotEnlisted() } } } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/TestDialect.cs b/src/NHibernate.Test/TestDialect.cs index f1e73367118..991035f5b65 100644 --- a/src/NHibernate.Test/TestDialect.cs +++ b/src/NHibernate.Test/TestDialect.cs @@ -72,5 +72,13 @@ public bool SupportsSqlType(SqlType sqlType) return false; } } + + /// + /// Some databases fail with dependent transaction, typically when their driver tries to access the transaction + /// state from its two PC: the dependent transaction is meant to be disposed of before completing the actual + /// transaction, so it is usually disposed at this point, and its state cannot be read. (Drivers should always + /// clone transactions for avoiding this trouble.) + /// + public virtual bool SupportsDependentTransaction => true; } } diff --git a/src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs b/src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs index 61fede783bf..bfd6e4166fd 100644 --- a/src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs +++ b/src/NHibernate.Test/TestDialects/PostgreSQL83TestDialect.cs @@ -16,5 +16,11 @@ public override bool SupportsNullCharactersInUtfStrings { get { return false; } } + + /// + /// Npgsql does not clone the transaction in its context, and uses it in its prepare phase. When that was a + /// dependent transaction, it is then usually already disposed of, causing Npgsql to crash. + /// + public override bool SupportsDependentTransaction => false; } } diff --git a/src/NHibernate/AdoNet/ConnectionManager.cs b/src/NHibernate/AdoNet/ConnectionManager.cs index e9881c737fd..9ad6091a07c 100644 --- a/src/NHibernate/AdoNet/ConnectionManager.cs +++ b/src/NHibernate/AdoNet/ConnectionManager.cs @@ -456,7 +456,13 @@ public DbCommand CreateCommand() public void EnlistIfRequired(System.Transactions.Transaction transaction) { if (transaction == _currentSystemTransaction) + { + // Short-circuit after having stored the transaction : they may be equal, but not the same reference. + // And the previous one may be an already disposed dependent clone, in which case we need to update + // our reference. + _currentSystemTransaction = transaction; return; + } _currentSystemTransaction = transaction; diff --git a/src/NHibernate/Async/Transaction/AdoNetWithSystemTransactionFactory.cs b/src/NHibernate/Async/Transaction/AdoNetWithSystemTransactionFactory.cs index 599752d270c..91558147f86 100644 --- a/src/NHibernate/Async/Transaction/AdoNetWithSystemTransactionFactory.cs +++ b/src/NHibernate/Async/Transaction/AdoNetWithSystemTransactionFactory.cs @@ -16,7 +16,6 @@ using NHibernate.AdoNet; using NHibernate.Engine; using NHibernate.Engine.Transaction; -using NHibernate.Impl; using NHibernate.Util; namespace NHibernate.Transaction diff --git a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs index 9205521dd8d..f9f902bac35 100644 --- a/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs +++ b/src/NHibernate/Transaction/AdoNetWithSystemTransactionFactory.cs @@ -6,7 +6,6 @@ using NHibernate.AdoNet; using NHibernate.Engine; using NHibernate.Engine.Transaction; -using NHibernate.Impl; using NHibernate.Util; namespace NHibernate.Transaction @@ -186,6 +185,7 @@ public class SystemTransactionContext : ITransactionContext, IEnlistmentNotifica private readonly System.Transactions.Transaction _originalTransaction; private readonly ManualResetEventSlim _lock = new ManualResetEventSlim(true); private volatile bool _needCompletionLocking = true; + private bool _preparing; // Required for not locking the completion phase itself when locking session usages from concurrent threads. private readonly AsyncLocal _bypassLock = new AsyncLocal(); private readonly int _systemTransactionCompletionLockTimeout; @@ -236,7 +236,9 @@ public virtual void Wait() // Remove the block then throw. Unlock(); throw new HibernateException( - "Synchronization timeout for transaction completion. Either raise {Cfg.Environment.SystemTransactionCompletionLockTimeout}, or this may be a bug in NHibernate."); + $"Synchronization timeout for transaction completion. Either raise" + + $"{Cfg.Environment.SystemTransactionCompletionLockTimeout}, or check all scopes are properly" + + $"disposed and/or all direct System.Transaction.Current changes are properly managed."); } catch (HibernateException) { @@ -284,16 +286,44 @@ protected virtual void Unlock() { // Cloned transaction is not disposed "unexpectedly", its status is accessible till context disposal. var status = EnlistedTransaction.TransactionInformation.Status; - if (status != TransactionStatus.Active) + if (status != TransactionStatus.Active || _preparing) return status; - // The clone status can be out of date when active, check the original one (which could be disposed if - // the clone is out of date). + // The clone status can be out of date when active and not in prepare phase, in case of rollback or + // dependent clone usage. + // In such case the original transaction is already disposed, and trying to check its status will + // trigger a dispose exception. return _originalTransaction.TransactionInformation.Status; } catch (ObjectDisposedException ode) { - _logger.Warn("Enlisted transaction status was wrongly active, original transaction being already disposed. Will assume neither active nor committed.", ode); + // For ruling out the dependent clone case when possible, we check if the current transaction is + // equal to the context one (System.Transactions.Transaction does override equality for this), and + // in such case, we check the state of the current transaction instead. (The state of the current + // transaction if equal can only be the same, but it will be inaccessible in case of rollback, due + // to the current transaction being already disposed.) + // The current transaction may not be reachable during 2PC phases and transaction completion events, + // but in such cases the context transaction is either no more active or in prepare phase, which is + // already covered by _preparing test. + try + { + var currentTransaction = System.Transactions.Transaction.Current; + if (!ReferenceEquals(currentTransaction, _originalTransaction) && + currentTransaction == EnlistedTransaction) + return currentTransaction.TransactionInformation.Status; + } + catch (ObjectDisposedException) + { + // Just ignore that one, no use to log two dispose exceptions which are indeed the same. + } + catch (InvalidOperationException ioe) + { + _logger.Warn("Attempting to dodge a disposed transaction trouble, current" + + "transaction was unreachable.", ioe); + } + + _logger.Warn("Enlisted transaction status is maybe wrongly active, original " + + "transaction being already disposed. Will assume neither active nor committed.", ode); return null; } } @@ -309,6 +339,7 @@ protected virtual void Unlock() /// The object for notifying the prepare phase outcome. public virtual void Prepare(PreparingEnlistment preparingEnlistment) { + _preparing = true; using (_session.BeginContext()) { try @@ -342,10 +373,12 @@ public virtual void Prepare(PreparingEnlistment preparingEnlistment) Lock(); _logger.Debug("Prepared for system transaction"); + _preparing = false; preparingEnlistment.Prepared(); } catch (Exception exception) { + _preparing = false; _logger.Error("System transaction prepare phase failed", exception); try { @@ -403,7 +436,7 @@ protected virtual void ProcessSecondPhase(Enlistment enlistment, bool? success) /// /// Handle the transaction completion. Notify of the end of the /// transaction. Notify end of transaction to the session and to - /// if any. Close sessions requiring it then cleanup transaction contextes and then blocked + /// if any. Close sessions requiring it then cleanup transaction contexts and then blocked /// threads. /// /// if the transaction is committed,