Skip to content

Commit 0df5b9d

Browse files
oskarboskarb
committed
Fix for NH-2128: Avoid firing the AfterTransactionCompletion notification twice when using distributed transactions and user-supplied connections.
The fix is by stopping the ConnectionManager from sometimes calling session.AfterTransactionCompletion(wasSuccessful, null). Such notifications should come from the transaction itself (as it already does) and in some cases from inside the session itself. Longer explanation in the bug report. This also works to simplify the complicated bidirectional relationship between SessionImpl and ConnectionManager. The session should rely on the connection manager, but not the other way around.
1 parent 21bb612 commit 0df5b9d

File tree

6 files changed

+194
-85
lines changed

6 files changed

+194
-85
lines changed

‎src/NHibernate.Test/NHibernate.Test.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,7 @@
10831083
<Compile Include="ReadOnly\StudentDto.cs" />
10841084
<Compile Include="ReadOnly\TextHolder.cs" />
10851085
<Compile Include="ReadOnly\VersionedNode.cs" />
1086+
<Compile Include="RecordingInterceptor.cs" />
10861087
<Compile Include="Stateless\Contact.cs" />
10871088
<Compile Include="Stateless\Country.cs" />
10881089
<Compile Include="Stateless\FetchingLazyCollections\LazyCollectionFetchTests.cs" />

‎src/NHibernate.Test/RecordingInterceptor.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
namespace NHibernate.Test
2+
{
3+
public class RecordingInterceptor : EmptyInterceptor
4+
{
5+
public int afterTransactionBeginCalled;
6+
public int afterTransactionCompletionCalled;
7+
public int beforeTransactionCompletionCalled;
8+
9+
public override void AfterTransactionBegin(ITransaction tx)
10+
{
11+
afterTransactionBeginCalled++;
12+
}
13+
14+
public override void AfterTransactionCompletion(ITransaction tx)
15+
{
16+
afterTransactionCompletionCalled++;
17+
}
18+
19+
public override void BeforeTransactionCompletion(ITransaction tx)
20+
{
21+
beforeTransactionCompletionCalled++;
22+
}
23+
}
24+
}

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

Lines changed: 93 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
using System;
12
using System.Collections;
3+
using System.Data;
24
using System.Transactions;
35
using NUnit.Framework;
46

@@ -12,32 +14,11 @@ protected override IList Mappings
1214
get { return new string[] {}; }
1315
}
1416

15-
public class RecordingInterceptor : EmptyInterceptor
16-
{
17-
public int afterTransactionBeginCalled;
18-
public int afterTransactionCompletionCalled;
19-
public int beforeTransactionCompletionCalled;
20-
21-
public override void AfterTransactionBegin(ITransaction tx)
22-
{
23-
afterTransactionBeginCalled++;
24-
}
25-
26-
public override void AfterTransactionCompletion(ITransaction tx)
27-
{
28-
afterTransactionCompletionCalled++;
29-
}
30-
31-
public override void BeforeTransactionCompletion(ITransaction tx)
32-
{
33-
beforeTransactionCompletionCalled++;
34-
}
35-
}
3617

3718
[Test]
3819
public void NoTransaction()
3920
{
40-
RecordingInterceptor interceptor = new RecordingInterceptor();
21+
var interceptor = new RecordingInterceptor();
4122
using (sessions.OpenSession(interceptor))
4223
{
4324
Assert.AreEqual(0, interceptor.afterTransactionBeginCalled);
@@ -49,7 +30,7 @@ public void NoTransaction()
4930
[Test]
5031
public void AfterBegin()
5132
{
52-
RecordingInterceptor interceptor = new RecordingInterceptor();
33+
var interceptor = new RecordingInterceptor();
5334
using (new TransactionScope())
5435
using (sessions.OpenSession(interceptor))
5536
{
@@ -62,9 +43,9 @@ public void AfterBegin()
6243
[Test]
6344
public void Complete()
6445
{
65-
RecordingInterceptor interceptor = new RecordingInterceptor();
46+
var interceptor = new RecordingInterceptor();
6647
ISession session;
67-
using(TransactionScope scope = new TransactionScope())
48+
using(var scope = new TransactionScope())
6849
{
6950
session = sessions.OpenSession(interceptor);
7051
scope.Complete();
@@ -78,7 +59,7 @@ public void Complete()
7859
[Test]
7960
public void Rollback()
8061
{
81-
RecordingInterceptor interceptor = new RecordingInterceptor();
62+
var interceptor = new RecordingInterceptor();
8263
using (new TransactionScope())
8364
using (sessions.OpenSession(interceptor))
8465
{
@@ -126,5 +107,91 @@ public void OneTransactionScopesInsideOneSession()
126107
Assert.AreEqual(1, interceptor.beforeTransactionCompletionCalled);
127108
Assert.AreEqual(1, interceptor.afterTransactionCompletionCalled);
128109
}
110+
111+
112+
[Description("NH2128")]
113+
[Theory]
114+
public void ShouldNotifyAfterDistributedTransaction(bool doCommit)
115+
{
116+
// Note: For distributed transaction, calling Close() on the session isn't
117+
// supported, so we don't need to test that scenario.
118+
119+
if (!doCommit)
120+
Assert.Ignore("Rollback on distributed transaction doubles the number of calls to AfterTransactionCompletion - see NH-3572.");
121+
122+
var interceptor = new RecordingInterceptor();
123+
ISession s1 = null;
124+
ISession s2 = null;
125+
126+
using (var tx = new TransactionScope())
127+
{
128+
try
129+
{
130+
s1 = OpenSession(interceptor);
131+
s2 = OpenSession(interceptor);
132+
133+
s1.CreateCriteria<object>().List();
134+
s2.CreateCriteria<object>().List();
135+
}
136+
finally
137+
{
138+
if (s1 != null)
139+
s1.Dispose();
140+
if (s2 != null)
141+
s2.Dispose();
142+
}
143+
144+
if (doCommit)
145+
tx.Complete();
146+
}
147+
148+
Assert.That(s1.IsOpen, Is.False);
149+
Assert.That(s2.IsOpen, Is.False);
150+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(2));
151+
}
152+
153+
154+
[Description("NH2128")]
155+
[Theory]
156+
public void ShouldNotifyAfterDistributedTransactionWithOwnConnection(bool doCommit)
157+
{
158+
// Note: For distributed transaction, calling Close() on the session isn't
159+
// supported, so we don't need to test that scenario.
160+
161+
var interceptor = new RecordingInterceptor();
162+
ISession s1 = null;
163+
ISession s2 = null;
164+
165+
using (var tx = new TransactionScope())
166+
{
167+
using (IDbConnection ownConnection1 = sessions.ConnectionProvider.GetConnection())
168+
using (IDbConnection ownConnection2 = sessions.ConnectionProvider.GetConnection())
169+
{
170+
try
171+
{
172+
s1 = sessions.OpenSession(ownConnection1, interceptor);
173+
s2 = sessions.OpenSession(ownConnection2, interceptor);
174+
175+
s1.CreateCriteria<object>().List();
176+
s2.CreateCriteria<object>().List();
177+
}
178+
finally
179+
{
180+
if (s1 != null)
181+
s1.Dispose();
182+
if (s2 != null)
183+
s2.Dispose();
184+
}
185+
186+
if (doCommit)
187+
tx.Complete();
188+
}
189+
}
190+
191+
Assert.That(s1.IsOpen, Is.False);
192+
Assert.That(s2.IsOpen, Is.False);
193+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(2));
194+
}
195+
129196
}
130197
}

‎src/NHibernate.Test/TransactionTest/TransactionNotificationFixture.cs

Lines changed: 64 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
using System;
21
using System.Collections;
2+
using System.Data;
33
using NUnit.Framework;
44

55
namespace NHibernate.Test.TransactionTest
@@ -12,77 +12,105 @@ protected override IList Mappings
1212
get { return new string[] {}; }
1313
}
1414

15-
public class RecordingInterceptor : EmptyInterceptor
16-
{
17-
public bool afterTransactionBeginCalled;
18-
public bool afterTransactionCompletionCalled;
19-
public bool beforeTransactionCompletionCalled;
20-
21-
public override void AfterTransactionBegin(ITransaction tx)
22-
{
23-
afterTransactionBeginCalled = true;
24-
}
25-
26-
public override void AfterTransactionCompletion(ITransaction tx)
27-
{
28-
afterTransactionCompletionCalled = true;
29-
}
30-
31-
public override void BeforeTransactionCompletion(ITransaction tx)
32-
{
33-
beforeTransactionCompletionCalled = true;
34-
}
35-
}
3615

3716
[Test]
3817
public void NoTransaction()
3918
{
40-
RecordingInterceptor interceptor = new RecordingInterceptor();
19+
var interceptor = new RecordingInterceptor();
4120
using (sessions.OpenSession(interceptor))
4221
{
43-
Assert.IsFalse(interceptor.afterTransactionBeginCalled);
44-
Assert.IsFalse(interceptor.beforeTransactionCompletionCalled);
45-
Assert.IsFalse(interceptor.afterTransactionCompletionCalled);
22+
Assert.That(interceptor.afterTransactionBeginCalled, Is.EqualTo(0));
23+
Assert.That(interceptor.beforeTransactionCompletionCalled, Is.EqualTo(0));
24+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(0));
4625
}
4726
}
4827

4928
[Test]
5029
public void AfterBegin()
5130
{
52-
RecordingInterceptor interceptor = new RecordingInterceptor();
31+
var interceptor = new RecordingInterceptor();
5332
using (ISession session = sessions.OpenSession(interceptor))
5433
using (session.BeginTransaction())
5534
{
56-
Assert.IsTrue(interceptor.afterTransactionBeginCalled);
57-
Assert.IsFalse(interceptor.beforeTransactionCompletionCalled);
58-
Assert.IsFalse(interceptor.afterTransactionCompletionCalled);
35+
Assert.That(interceptor.afterTransactionBeginCalled, Is.EqualTo(1));
36+
Assert.That(interceptor.beforeTransactionCompletionCalled, Is.EqualTo(0));
37+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(0));
5938
}
6039
}
6140

6241
[Test]
6342
public void Commit()
6443
{
65-
RecordingInterceptor interceptor = new RecordingInterceptor();
44+
var interceptor = new RecordingInterceptor();
6645
using (ISession session = sessions.OpenSession(interceptor))
6746
{
6847
ITransaction tx = session.BeginTransaction();
6948
tx.Commit();
70-
Assert.IsTrue(interceptor.beforeTransactionCompletionCalled);
71-
Assert.IsTrue(interceptor.afterTransactionCompletionCalled);
49+
Assert.That(interceptor.afterTransactionBeginCalled, Is.EqualTo(1));
50+
Assert.That(interceptor.beforeTransactionCompletionCalled, Is.EqualTo(1));
51+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(1));
7252
}
7353
}
7454

7555
[Test]
7656
public void Rollback()
7757
{
78-
RecordingInterceptor interceptor = new RecordingInterceptor();
58+
var interceptor = new RecordingInterceptor();
7959
using (ISession session = sessions.OpenSession(interceptor))
8060
{
8161
ITransaction tx = session.BeginTransaction();
8262
tx.Rollback();
83-
Assert.IsFalse(interceptor.beforeTransactionCompletionCalled);
84-
Assert.IsTrue(interceptor.afterTransactionCompletionCalled);
63+
Assert.That(interceptor.afterTransactionBeginCalled, Is.EqualTo(1));
64+
Assert.That(interceptor.beforeTransactionCompletionCalled, Is.EqualTo(0));
65+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(1));
8566
}
8667
}
68+
69+
70+
[Theory]
71+
[Description("NH2128")]
72+
public void ShouldNotifyAfterTransaction(bool usePrematureClose)
73+
{
74+
var interceptor = new RecordingInterceptor();
75+
ISession s;
76+
77+
using (s = OpenSession(interceptor))
78+
using (s.BeginTransaction())
79+
{
80+
s.CreateCriteria<object>().List();
81+
82+
// Call session close while still inside transaction?
83+
if (usePrematureClose)
84+
s.Close();
85+
}
86+
87+
Assert.That(s.IsOpen, Is.False);
88+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(1));
89+
}
90+
91+
92+
[Description("NH2128")]
93+
[Theory]
94+
public void ShouldNotifyAfterTransactionWithOwnConnection(bool usePrematureClose)
95+
{
96+
var interceptor = new RecordingInterceptor();
97+
ISession s;
98+
99+
using (IDbConnection ownConnection = sessions.ConnectionProvider.GetConnection())
100+
{
101+
using (s = sessions.OpenSession(ownConnection, interceptor))
102+
using (s.BeginTransaction())
103+
{
104+
s.CreateCriteria<object>().List();
105+
106+
// Call session close while still inside transaction?
107+
if (usePrematureClose)
108+
s.Close();
109+
}
110+
}
111+
112+
Assert.That(s.IsOpen, Is.False);
113+
Assert.That(interceptor.afterTransactionCompletionCalled, Is.EqualTo(1));
114+
}
87115
}
88116
}

‎src/NHibernate/AdoNet/ConnectionManager.cs

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -153,31 +153,20 @@ private void DisconnectOwnConnection()
153153
CloseConnection();
154154
}
155155

156-
public IDbConnection Disconnect() {
157-
if (IsInActiveTransaction)
158-
throw new InvalidOperationException("Disconnect cannot be called while a transaction is in progress.");
159-
try
156+
public IDbConnection Disconnect()
157+
{
158+
if (IsInActiveTransaction)
159+
throw new InvalidOperationException("Disconnect cannot be called while a transaction is in progress.");
160+
161+
if (!ownConnection)
160162
{
161-
if (!ownConnection)
162-
{
163-
return DisconnectSuppliedConnection();
164-
}
165-
else
166-
{
167-
DisconnectOwnConnection();
168-
ownConnection = false;
169-
return null;
170-
}
163+
return DisconnectSuppliedConnection();
171164
}
172-
finally
165+
else
173166
{
174-
// Ensure that AfterTransactionCompletion gets called since
175-
// it takes care of the locks and cache.
176-
if (!IsInActiveTransaction)
177-
{
178-
// We don't know the state of the transaction
179-
session.AfterTransactionCompletion(false, null);
180-
}
167+
DisconnectOwnConnection();
168+
ownConnection = false;
169+
return null;
181170
}
182171
}
183172

@@ -340,7 +329,6 @@ public ITransaction Transaction
340329
public void AfterNonTransactionalQuery(bool success)
341330
{
342331
log.Debug("after autocommit");
343-
session.AfterTransactionCompletion(success, null);
344332
}
345333

346334
private bool IsAfterTransactionRelease

0 commit comments

Comments
 (0)