Skip to content

Commit 5d1fbdf

Browse files
fredericDelaportefredericDelaporte
committed
NH-2176 - best attempt I can imagine for fixing current transaction handling.
1 parent 88491fc commit 5d1fbdf

File tree

5 files changed

+142
-109
lines changed

5 files changed

+142
-109
lines changed

‎src/NHibernate.Test/NHSpecificTest/DtcFailures/DtcFailuresFixture.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public void TransactionInsertWithRollBackTask()
169169
}
170170
}
171171

172-
[Test, Ignore("Not fixed.")]
172+
[Test/*, Ignore("Not fixed.")*/]
173173
[Description(@"Two session in two txscope
174174
(without an explicit NH transaction and without an explicit flush)
175175
and with a rollback in the second dtc and a ForceRollback outside nh-session-scope.")]

‎src/NHibernate.Test/NHSpecificTest/NH1632/Fixture.cs

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ protected override void Configure(Configuration configuration)
2020
{
2121
configuration
2222
.SetProperty(Environment.UseSecondLevelCache, "true")
23-
.SetProperty(Environment.CacheProvider, typeof (HashtableCacheProvider).AssemblyQualifiedName);
23+
.SetProperty(Environment.CacheProvider, typeof(HashtableCacheProvider).AssemblyQualifiedName);
2424
}
2525

2626
[Test]
@@ -40,9 +40,9 @@ public void When_using_DTC_HiLo_knows_to_create_isolated_DTC_transaction()
4040
var generator = sessions.GetIdentifierGenerator(typeof(Person).FullName);
4141
Assert.That(generator, Is.InstanceOf<TableHiLoGenerator>());
4242

43-
using(var session = sessions.OpenSession())
43+
using (var session = sessions.OpenSession())
4444
{
45-
var id = generator.Generate((ISessionImplementor) session, new Person());
45+
var id = generator.Generate((ISessionImplementor)session, new Person());
4646
}
4747

4848
// intentionally dispose without committing
@@ -56,7 +56,7 @@ public void When_using_DTC_HiLo_knows_to_create_isolated_DTC_transaction()
5656
scalar2 = command.ExecuteScalar();
5757
}
5858

59-
Assert.AreNotEqual(scalar1, scalar2,"HiLo must run with in its own transaction");
59+
Assert.AreNotEqual(scalar1, scalar2, "HiLo must run with in its own transaction");
6060
}
6161

6262
[Test]
@@ -84,43 +84,51 @@ public void When_commiting_items_in_DTC_transaction_will_add_items_to_2nd_level_
8484
{
8585
using (var s = sessions.OpenSession())
8686
{
87-
s.Save(new Nums {ID = 29, NumA = 1, NumB = 3});
87+
s.Save(new Nums { ID = 29, NumA = 1, NumB = 3 });
8888
}
8989
tx.Complete();
9090
}
91-
92-
using (var tx = new TransactionScope())
91+
try
9392
{
94-
using (var s = sessions.OpenSession())
93+
94+
using (var tx = new TransactionScope())
9595
{
96-
var nums = s.Load<Nums>(29);
97-
Assert.AreEqual(1, nums.NumA);
98-
Assert.AreEqual(3, nums.NumB);
96+
using (var s = sessions.OpenSession())
97+
{
98+
var nums = s.Load<Nums>(29);
99+
Assert.AreEqual(1, nums.NumA);
100+
Assert.AreEqual(3, nums.NumB);
101+
}
102+
tx.Complete();
99103
}
100-
tx.Complete();
101-
}
102104

103-
//closing the connection to ensure we can't really use it.
104-
var connection = sessions.ConnectionProvider.GetConnection();
105-
sessions.ConnectionProvider.CloseConnection(connection);
105+
//closing the connection to ensure we can't really use it.
106+
var connection = sessions.ConnectionProvider.GetConnection();
107+
sessions.ConnectionProvider.CloseConnection(connection);
108+
// The session is supposed to succeed because the second level cache should have the
109+
// entity to load, allowing the session to not use the connection at all.
106110

107-
using (var tx = new TransactionScope())
108-
{
109-
using (var s = sessions.OpenSession(connection))
111+
using (var tx = new TransactionScope())
110112
{
111-
var nums = s.Load<Nums>(29);
112-
Assert.AreEqual(1, nums.NumA);
113-
Assert.AreEqual(3, nums.NumB);
113+
using (var s = sessions.OpenSession(connection))
114+
{
115+
Nums nums = null;
116+
Assert.DoesNotThrow(() => nums = s.Load<Nums>(29), "Failed loading entity from second level cache.");
117+
Assert.AreEqual(1, nums.NumA);
118+
Assert.AreEqual(3, nums.NumB);
119+
}
120+
tx.Complete();
114121
}
115-
tx.Complete();
116122
}
117-
118-
using (var s = sessions.OpenSession())
119-
using (var tx = s.BeginTransaction())
123+
finally
120124
{
121-
var nums = s.Load<Nums>(29);
122-
s.Delete(nums);
123-
tx.Commit();
125+
using (var s = sessions.OpenSession())
126+
using (var tx = s.BeginTransaction())
127+
{
128+
var nums = s.Load<Nums>(29);
129+
s.Delete(nums);
130+
tx.Commit();
131+
}
124132
}
125133
}
126134

‎src/NHibernate.Test/NHSpecificTest/NH2420/Fixture.cs

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Data.Odbc;
44
using System.Data.SqlClient;
55
using System.Configuration;
6+
using System.Threading;
67
using System.Transactions;
78
using NHibernate.Dialect;
89
using NHibernate.Driver;
@@ -55,41 +56,58 @@ public void ShouldBeAbleToReleaseSuppliedConnectionAfterDistributedTransaction()
5556
{
5657
string connectionString = FetchConnectionStringFromConfiguration();
5758
ISession s;
58-
using (var ts = new TransactionScope())
59+
DbConnection connection = null;
60+
try
5961
{
60-
// Enlisting DummyEnlistment as a durable resource manager will start
61-
// a DTC transaction
62-
System.Transactions.Transaction.Current.EnlistDurable(
63-
DummyEnlistment.Id,
64-
new DummyEnlistment(),
65-
EnlistmentOptions.None);
62+
using (var ts = new TransactionScope())
63+
{
64+
// Enlisting DummyEnlistment as a durable resource manager will start
65+
// a DTC transaction
66+
System.Transactions.Transaction.Current.EnlistDurable(
67+
DummyEnlistment.Id,
68+
new DummyEnlistment(),
69+
EnlistmentOptions.None);
6670

67-
DbConnection connection;
68-
if (sessions.ConnectionProvider.Driver.GetType() == typeof(OdbcDriver))
69-
connection = new OdbcConnection(connectionString);
70-
else
71-
connection = new SqlConnection(connectionString);
71+
//DbConnection connection;
72+
if (sessions.ConnectionProvider.Driver.GetType() == typeof(OdbcDriver))
73+
connection = new OdbcConnection(connectionString);
74+
else
75+
connection = new SqlConnection(connectionString);
7276

73-
using (connection)
74-
{
77+
/*using (connection)
78+
{*/
7579
connection.Open();
7680
using (s = Sfi.OpenSession(connection))
7781
{
78-
s.Save(new MyTable { String = "hello!" });
82+
s.Save(new MyTable {String = "hello!"});
7983
}
80-
connection.Close();
81-
}
84+
// The ts disposal may try to flush the session, which, depending on the native generator implementation
85+
// for current dialect, may have something to do and will then try to use the supplied connection
86+
// => flaky test, failing for dialects not mandating an immediate insert on native generator save.
87+
// Delaying the connection disposal to after ts disposal.
88+
/* connection.Close();
89+
}*/
8290

83-
ts.Complete();
91+
ts.Complete();
92+
}
93+
}
94+
finally
95+
{
96+
connection?.Dispose();
8497
}
8598

99+
// Here it appears the second phase of the 2PC is not guaranteed to be executed
100+
// before exiting transaction scope disposal... So long for the current pattern,
101+
// it looks doomed. Sleeping here allow to keep that test succeeding by letting
102+
// enough time for the second phase to disconnect the session.
103+
//Thread.Sleep(100);
104+
86105
// Prior to the patch, an InvalidOperationException exception would occur in the
87106
// TransactionCompleted delegate at this point with the message, "Disconnect cannot
88107
// be called while a transaction is in progress". Although the exception can be
89108
// seen reported in the IDE, NUnit fails to see it. The TransactionCompleted event
90109
// fires *after* the transaction is committed and so it doesn't affect the success
91110
// of the transaction.
92-
93111
Assert.That(s.IsConnected, Is.False);
94112
Assert.That(((ISessionImplementor)s).ConnectionManager.IsConnected, Is.False);
95113
Assert.That(((ISessionImplementor)s).IsClosed, Is.True);

‎src/NHibernate/AdoNet/ConnectionManager.cs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using System.Data.Common;
44
using System.Runtime.Serialization;
55
using System.Security;
6-
using System.Security.Permissions;
76

87
using NHibernate.Engine;
98

@@ -30,6 +29,10 @@ public interface Callback
3029

3130
[NonSerialized]
3231
private DbConnection connection;
32+
[NonSerialized]
33+
private DbConnection _dtcBackupConnection;
34+
[NonSerialized]
35+
private System.Transactions.Transaction _connectionAmbientTransaction;
3336
// Whether we own the connection, i.e. connect and disconnect automatically.
3437
private bool ownConnection;
3538

@@ -185,6 +188,7 @@ public DbConnection GetConnection()
185188
if (ownConnection)
186189
{
187190
connection = Factory.ConnectionProvider.GetConnection();
191+
_connectionAmbientTransaction = System.Transactions.Transaction.Current;
188192
if (Factory.Statistics.IsStatisticsEnabled)
189193
{
190194
Factory.StatisticsImplementor.Connect();
@@ -380,10 +384,21 @@ public IBatcher Batcher
380384
get { return batcher; }
381385
}
382386

387+
public bool RequireExplicitEnlistment
388+
=> connection != null && _connectionAmbientTransaction != System.Transactions.Transaction.Current;
389+
383390
public IDisposable FlushingFromDtcTransaction
384391
{
385392
get
386393
{
394+
if (ownConnection)
395+
{
396+
if (Batcher.HasOpenResources)
397+
throw new InvalidOperationException("Batcher still has opened ressources at time of Flush from DTC.");
398+
// Swap out current connection for avoiding using it concurrently to its own 2PC
399+
_dtcBackupConnection = connection;
400+
connection = null;
401+
}
387402
flushingFromDtcTransaction = true;
388403
return new StopFlushingFromDtcTransaction(this);
389404
}
@@ -401,6 +416,15 @@ public StopFlushingFromDtcTransaction(ConnectionManager manager)
401416
public void Dispose()
402417
{
403418
manager.flushingFromDtcTransaction = false;
419+
420+
if (manager.ownConnection)
421+
{
422+
// Release the connection potentially acquired for flushing from DTC.
423+
manager.DisconnectOwnConnection();
424+
// Swap back current connection
425+
manager.connection = manager._dtcBackupConnection;
426+
manager._dtcBackupConnection = null;
427+
}
404428
}
405429
}
406430

‎src/NHibernate/Transaction/AdoNetWithDistributedTransactionFactory.cs

Lines changed: 42 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -26,48 +26,24 @@ public void EnlistInDistributedTransactionIfNeeded(ISessionImplementor session)
2626
{
2727
if (session.TransactionContext != null)
2828
return;
29-
30-
if (System.Transactions.Transaction.Current == null)
29+
30+
var transaction = System.Transactions.Transaction.Current;
31+
if (transaction == null)
3132
return;
32-
33-
var transactionContext = new DistributedTransactionContext(session,
34-
System.Transactions.Transaction.Current);
33+
34+
if (session.ConnectionManager.RequireExplicitEnlistment)
35+
{
36+
// Will fail if the connection is already enlisted in another not yet completed transaction.
37+
// Probable case: nested transaction scope. Supporting this could be done by releasing the
38+
// connection instead of enlisting.
39+
session.Connection.EnlistTransaction(transaction);
40+
}
41+
var transactionContext = new DistributedTransactionContext(session, transaction);
3542
session.TransactionContext = transactionContext;
3643
logger.DebugFormat("enlisted into DTC transaction: {0}",
3744
transactionContext.AmbientTransation.IsolationLevel);
3845
session.AfterTransactionBegin(null);
3946

40-
TransactionCompletedEventHandler handler = null;
41-
42-
handler = delegate(object sender, TransactionEventArgs e)
43-
{
44-
using (new SessionIdLoggingContext(session.SessionId))
45-
{
46-
((DistributedTransactionContext) session.TransactionContext).IsInActiveTransaction = false;
47-
48-
bool wasSuccessful = false;
49-
try
50-
{
51-
wasSuccessful = e.Transaction.TransactionInformation.Status
52-
== TransactionStatus.Committed;
53-
}
54-
catch (ObjectDisposedException ode)
55-
{
56-
logger.Warn("Completed transaction was disposed, assuming transaction rollback", ode);
57-
}
58-
session.AfterTransactionCompletion(wasSuccessful, null);
59-
if (transactionContext.ShouldCloseSessionOnDistributedTransactionCompleted)
60-
{
61-
session.CloseSessionFromDistributedTransaction();
62-
}
63-
session.TransactionContext = null;
64-
}
65-
66-
e.Transaction.TransactionCompleted -= handler;
67-
};
68-
69-
transactionContext.AmbientTransation.TransactionCompleted += handler;
70-
7147
transactionContext.AmbientTransation.EnlistVolatile(transactionContext,
7248
EnlistmentOptions.EnlistDuringPrepareRequired);
7349
}
@@ -138,37 +114,44 @@ void IEnlistmentNotification.Prepare(PreparingEnlistment preparingEnlistment)
138114
}
139115

140116
void IEnlistmentNotification.Commit(Enlistment enlistment)
141-
{
142-
using (new SessionIdLoggingContext(sessionImplementor.SessionId))
143-
{
144-
logger.Debug("committing DTC transaction");
145-
// we have nothing to do here, since it is the actual
146-
// DB connection that will commit the transaction
147-
enlistment.Done();
148-
IsInActiveTransaction = false;
149-
}
150-
}
117+
=> ProcessSecondPhase(enlistment, true);
151118

152119
void IEnlistmentNotification.Rollback(Enlistment enlistment)
153-
{
154-
using (new SessionIdLoggingContext(sessionImplementor.SessionId))
155-
{
156-
logger.Debug("rolled back DTC transaction");
157-
// Currently AfterTransactionCompletion is called by the handler for the TransactionCompleted event.
158-
//sessionImplementor.AfterTransactionCompletion(false, null);
159-
enlistment.Done();
160-
IsInActiveTransaction = false;
161-
}
162-
}
120+
=> ProcessSecondPhase(enlistment, false);
163121

164122
void IEnlistmentNotification.InDoubt(Enlistment enlistment)
123+
=> ProcessSecondPhase(enlistment, null);
124+
125+
private void ProcessSecondPhase(Enlistment enlistment, bool? success)
165126
{
166127
using (new SessionIdLoggingContext(sessionImplementor.SessionId))
167128
{
168-
sessionImplementor.AfterTransactionCompletion(false, null);
169-
logger.Debug("DTC transaction is in doubt");
170-
enlistment.Done();
129+
logger.Debug(success.HasValue
130+
? success.Value ? "committing DTC transaction" : "rolled back DTC transaction"
131+
: "DTC transaction is in doubt");
132+
// we have not much to do here, since it is the actual
133+
// DB connection that will commit/rollback the transaction
171134
IsInActiveTransaction = false;
135+
// In doubt means the transaction may get carried on successfully, but maybe one hour later, the
136+
// time for the failing durable ressource to come back online and tell. We won't wait for knowing,
137+
// so better be pessimist.
138+
var signalSuccess = success ?? false;
139+
// May fail by releasing the connection while the connection has its own second phase to do.
140+
// Since we can release connection before completing an ambient transaction, maybe it will never
141+
// fail, but here we are at the transaction completion stage, which is not documented for
142+
// supporting this. See next comment as for why we cannot do that within
143+
// TransactionCompletion event.
144+
sessionImplementor.AfterTransactionCompletion(signalSuccess, null);
145+
146+
if (sessionImplementor.TransactionContext.ShouldCloseSessionOnDistributedTransactionCompleted)
147+
{
148+
sessionImplementor.CloseSessionFromDistributedTransaction();
149+
}
150+
sessionImplementor.TransactionContext = null;
151+
152+
// Do not signal it is finished before having processed after-transaction actions, otherwise they
153+
// may be executed concurrently to next scope, which causes a bunch of issues.
154+
enlistment.Done();
172155
}
173156
}
174157

0 commit comments

Comments
 (0)