Skip to content

Commit 16c0a1f

Browse files
committed
HHH-9976 - JdbcResourceLocalTransactionCoordinatorImpl does not rollback on failure during #commit
1 parent 3d1a39f commit 16c0a1f

File tree

5 files changed

+42
-19
lines changed

5 files changed

+42
-19
lines changed

hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
/**
2323
* @author Andrea Boriero
24+
* @author Steve Ebersole
2425
*/
2526
public class TransactionImpl implements Transaction {
2627
private static final Logger LOG = CoreLogging.logger( TransactionImpl.class );
@@ -71,6 +72,13 @@ public void commit() {
7172
@Override
7273
public void rollback() {
7374
TransactionStatus status = transactionDriverControl.getStatus();
75+
if ( status == TransactionStatus.ROLLED_BACK || status == TransactionStatus.NOT_ACTIVE ) {
76+
// Allow rollback() calls on completed transactions, just no-op.
77+
LOG.debug( "rollback() called on an inactive transaction" );
78+
invalidate();
79+
return;
80+
}
81+
7482
if ( !status.canRollback() ) {
7583
throw new TransactionException( "Cannot rollback transaction in current status [" + status.name() + "]" );
7684
}

hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/JdbcResourceLocalTransactionCoordinatorImpl.java

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -222,19 +222,35 @@ protected void errorIfInvalid() {
222222

223223
@Override
224224
public void commit() {
225-
if ( rollbackOnly ) {
226-
throw new TransactionException( "Transaction was marked for rollback only; cannot commit" );
225+
try {
226+
if ( rollbackOnly ) {
227+
throw new TransactionException( "Transaction was marked for rollback only; cannot commit" );
228+
}
229+
230+
JdbcResourceLocalTransactionCoordinatorImpl.this.beforeCompletionCallback();
231+
jdbcResourceTransaction.commit();
232+
JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( true );
233+
}
234+
catch (RuntimeException e) {
235+
try {
236+
rollback();
237+
}
238+
catch (RuntimeException e2) {
239+
log.debug( "Encountered failure rolling back failed commit", e2 );;
240+
}
241+
throw e;
227242
}
228-
229-
JdbcResourceLocalTransactionCoordinatorImpl.this.beforeCompletionCallback();
230-
jdbcResourceTransaction.commit();
231-
JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( true );
232243
}
233244

234245
@Override
235246
public void rollback() {
236-
jdbcResourceTransaction.rollback();
237-
JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( false );
247+
if ( rollbackOnly || getStatus() == TransactionStatus.ACTIVE ) {
248+
rollbackOnly = false;
249+
jdbcResourceTransaction.rollback();
250+
JdbcResourceLocalTransactionCoordinatorImpl.this.afterCompletionCallback( false );
251+
}
252+
253+
// no-op otherwise.
238254
}
239255

240256
@Override

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/SynchronizationRegistryStandardImplTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public void testUserSynchronizationExceptions() {
106106
// exception in beforeCompletion
107107
registry.clearSynchronizations();
108108
registry = new SynchronizationRegistryStandardImpl();
109-
synchronization = new SynchronizationErrorImpl( false, true );
109+
synchronization = SynchronizationErrorImpl.forAfter();
110110
registry.registerSynchronization( synchronization );
111111
try {
112112
registry.notifySynchronizationsAfterTransactionCompletion( Status.STATUS_COMMITTED );

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jdbc/BasicJdbcTransactionTests.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,13 @@ public boolean shouldAutoJoinTransaction() {
8888
catch (TransactionException expected) {
8989
}
9090
finally {
91-
assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
92-
transactionCoordinator.getTransactionDriverControl().rollback();
91+
assertEquals( TransactionStatus.NOT_ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
9392
}
9493
}
9594

9695
@Test
9796
@SuppressWarnings("EmptyCatchBlock")
98-
public void testSynchronizationFailureMarksTransactionForRollbackOnly() {
97+
public void testSynchronizationFailure() {
9998
final TransactionCoordinatorOwnerTestingImpl owner = new TransactionCoordinatorOwnerTestingImpl();
10099
final JdbcResourceLocalTransactionCoordinatorBuilderImpl transactionCoordinatorBuilder =
101100
new JdbcResourceLocalTransactionCoordinatorBuilderImpl();
@@ -122,8 +121,10 @@ public boolean shouldAutoJoinTransaction() {
122121
catch (Exception expected) {
123122
}
124123
finally {
125-
assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
126-
transactionCoordinator.getTransactionDriverControl().rollback();
124+
assertEquals(
125+
TransactionStatus.NOT_ACTIVE,
126+
transactionCoordinator.getTransactionDriverControl().getStatus()
127+
);
127128
}
128129
}
129130
}

hibernate-core/src/test/java/org/hibernate/test/resource/transaction/jta/AbstractBasicJtaTestScenarios.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,13 @@ public void testMarkRollbackOnly() throws Exception {
401401
catch (TransactionException expected) {
402402
}
403403
finally {
404-
assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
405-
transactionCoordinator.getTransactionDriverControl().rollback();
404+
assertEquals( TransactionStatus.NOT_ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
406405
}
407406
}
408407

409408
@Test
410409
@SuppressWarnings("EmptyCatchBlock")
411-
public void testSynchronizationFailureMarksTransactionForRollbackOnly() throws Exception {
410+
public void testSynchronizationFailure() throws Exception {
412411
JtaTransactionCoordinatorImpl transactionCoordinator = new JtaTransactionCoordinatorImpl(
413412
transactionCoordinatorBuilder,
414413
owner,
@@ -434,8 +433,7 @@ public void testSynchronizationFailureMarksTransactionForRollbackOnly() throws E
434433
catch (Exception expected) {
435434
}
436435
finally {
437-
assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
438-
transactionCoordinator.getTransactionDriverControl().rollback();
436+
assertEquals( TransactionStatus.NOT_ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
439437
}
440438
}
441439
}

0 commit comments

Comments
 (0)