HHH-9976 - JdbcResourceLocalTransactionCoordinatorImpl does not rollback on failure during #beforeCompletionCallback

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

@@ -142,10 +142,19 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
 
 	private void beforeCompletionCallback() {
 		log.trace( "ResourceLocalTransactionCoordinatorImpl#beforeCompletionCallback" );
-		transactionCoordinatorOwner.beforeTransactionCompletion();
+		try {
-		synchronizationRegistry.notifySynchronizationsBeforeTransactionCompletion();
+			transactionCoordinatorOwner.beforeTransactionCompletion();
-		for ( TransactionObserver observer : observers ) {
+			synchronizationRegistry.notifySynchronizationsBeforeTransactionCompletion();
-			observer.beforeCompletion();
+			for ( TransactionObserver observer : observers ) {
+				observer.beforeCompletion();
+			}
+		}
+		catch (RuntimeException e) {
+			if ( physicalTransactionDelegate != null ) {
+				// should never happen that the physicalTransactionDelegate is null, but to be safe
+				physicalTransactionDelegate.markRollbackOnly();
+			}
+			throw e;
 		}
 	}
 

hibernate-core/src/test/java/org/hibernate/test/resource/common/SynchronizationErrorImpl.java

@@ -15,7 +15,19 @@ public class SynchronizationErrorImpl implements Synchronization {
 	private final boolean errorOnBefore;
 	private final boolean errorOnAfter;
 
-	public SynchronizationErrorImpl(boolean errorOnBefore, boolean errorOnAfter) {
+	public static SynchronizationErrorImpl forBefore() {
+		return new SynchronizationErrorImpl( true, false );
+	}
+
+	public static SynchronizationErrorImpl forAfter() {
+		return new SynchronizationErrorImpl( false, true );
+	}
+
+	public static SynchronizationErrorImpl forBoth() {
+		return new SynchronizationErrorImpl( true, true );
+	}
+
+	private SynchronizationErrorImpl(boolean errorOnBefore, boolean errorOnAfter) {
 		this.errorOnBefore = errorOnBefore;
 		this.errorOnAfter = errorOnAfter;
 	}

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

@@ -89,7 +89,7 @@ public class SynchronizationRegistryStandardImplTests {
 	public void testUserSynchronizationExceptions() {
 		// exception in beforeCompletion
 		SynchronizationRegistryStandardImpl registry = new SynchronizationRegistryStandardImpl();
-		Synchronization synchronization = new SynchronizationErrorImpl( true, false );
+		Synchronization synchronization = SynchronizationErrorImpl.forBefore();
 		registry.registerSynchronization( synchronization );
 		try {
 			registry.notifySynchronizationsBeforeTransactionCompletion();

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

@@ -6,6 +6,9 @@
  */
 package org.hibernate.test.resource.transaction.jdbc;
 
+import javax.transaction.Synchronization;
+
+import org.hibernate.HibernateException;
 import org.hibernate.TransactionException;
 import org.hibernate.resource.transaction.TransactionCoordinator;
 import org.hibernate.resource.transaction.TransactionCoordinatorBuilder;
@@ -13,6 +16,7 @@ import org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLoca
 import org.hibernate.resource.transaction.spi.TransactionStatus;
 
 import org.hibernate.test.resource.common.SynchronizationCollectorImpl;
+import org.hibernate.test.resource.common.SynchronizationErrorImpl;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -88,4 +92,38 @@ public class BasicJdbcTransactionTests {
 			transactionCoordinator.getTransactionDriverControl().rollback();
 		}
 	}
+
+	@Test
+	@SuppressWarnings("EmptyCatchBlock")
+	public void testSynchronizationFailureMarksTransactionForRollbackOnly() {
+		final TransactionCoordinatorOwnerTestingImpl owner = new TransactionCoordinatorOwnerTestingImpl();
+		final JdbcResourceLocalTransactionCoordinatorBuilderImpl transactionCoordinatorBuilder =
+				new JdbcResourceLocalTransactionCoordinatorBuilderImpl();
+
+		final TransactionCoordinator transactionCoordinator = transactionCoordinatorBuilder.buildTransactionCoordinator(
+				owner,
+				new TransactionCoordinatorBuilder.TransactionCoordinatorOptions() {
+					@Override
+					public boolean shouldAutoJoinTransaction() {
+						return false;
+					}
+				}
+		);
+
+		assertEquals( TransactionStatus.NOT_ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
+		transactionCoordinator.getLocalSynchronizations().registerSynchronization( SynchronizationErrorImpl.forBefore() );
+
+		transactionCoordinator.getTransactionDriverControl().begin();
+		assertEquals( TransactionStatus.ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
+
+		try {
+			transactionCoordinator.getTransactionDriverControl().commit();
+		}
+		catch (Exception expected) {
+		}
+		finally {
+			assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
+			transactionCoordinator.getTransactionDriverControl().rollback();
+		}
+	}
 }

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

@@ -7,15 +7,23 @@
 package org.hibernate.test.resource.transaction.jta;
 
 import javax.transaction.Status;
+import javax.transaction.Synchronization;
 import javax.transaction.SystemException;
 import javax.transaction.TransactionManager;
 
+import org.hibernate.HibernateException;
+import org.hibernate.TransactionException;
+import org.hibernate.resource.transaction.TransactionCoordinator;
+import org.hibernate.resource.transaction.TransactionCoordinatorBuilder;
 import org.hibernate.resource.transaction.TransactionRequiredForJoinException;
+import org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorBuilderImpl;
 import org.hibernate.resource.transaction.backend.jta.internal.JtaTransactionCoordinatorBuilderImpl;
 import org.hibernate.resource.transaction.backend.jta.internal.JtaTransactionCoordinatorImpl;
 import org.hibernate.resource.transaction.backend.jta.internal.synchronization.SynchronizationCallbackCoordinatorTrackingImpl;
+import org.hibernate.resource.transaction.spi.TransactionStatus;
 
 import org.hibernate.test.resource.common.SynchronizationCollectorImpl;
+import org.hibernate.test.resource.common.SynchronizationErrorImpl;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -359,4 +367,75 @@ public abstract class AbstractBasicJtaTestScenarios {
 
 		tm.rollback();
 	}
+
+	@Test
+	@SuppressWarnings("EmptyCatchBlock")
+	public void testMarkRollbackOnly() throws Exception {
+		JtaTransactionCoordinatorImpl transactionCoordinator = new JtaTransactionCoordinatorImpl(
+				transactionCoordinatorBuilder,
+				owner,
+				true,
+				JtaPlatformStandardTestingImpl.INSTANCE,
+				preferUserTransactions(),
+				true
+		);
+
+		// pre conditions
+		final TransactionManager tm = JtaPlatformStandardTestingImpl.INSTANCE.transactionManager();
+		assertEquals( Status.STATUS_NO_TRANSACTION, tm.getStatus() );
+
+		assertEquals( TransactionStatus.NOT_ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
+
+		transactionCoordinator.getTransactionDriverControl().begin();
+		assertEquals( TransactionStatus.ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
+
+		transactionCoordinator.getTransactionDriverControl().markRollbackOnly();
+		assertEquals(
+				TransactionStatus.MARKED_ROLLBACK,
+				transactionCoordinator.getTransactionDriverControl().getStatus()
+		);
+
+		try {
+			transactionCoordinator.getTransactionDriverControl().commit();
+		}
+		catch (TransactionException expected) {
+		}
+		finally {
+			assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
+			transactionCoordinator.getTransactionDriverControl().rollback();
+		}
+	}
+
+	@Test
+	@SuppressWarnings("EmptyCatchBlock")
+	public void testSynchronizationFailureMarksTransactionForRollbackOnly() throws Exception {
+		JtaTransactionCoordinatorImpl transactionCoordinator = new JtaTransactionCoordinatorImpl(
+				transactionCoordinatorBuilder,
+				owner,
+				true,
+				JtaPlatformStandardTestingImpl.INSTANCE,
+				preferUserTransactions(),
+				true
+		);
+
+		// pre conditions
+		final TransactionManager tm = JtaPlatformStandardTestingImpl.INSTANCE.transactionManager();
+		assertEquals( Status.STATUS_NO_TRANSACTION, tm.getStatus() );
+
+		assertEquals( TransactionStatus.NOT_ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
+		transactionCoordinator.getLocalSynchronizations().registerSynchronization( SynchronizationErrorImpl.forBefore() );
+
+		transactionCoordinator.getTransactionDriverControl().begin();
+		assertEquals( TransactionStatus.ACTIVE, transactionCoordinator.getTransactionDriverControl().getStatus() );
+
+		try {
+			transactionCoordinator.getTransactionDriverControl().commit();
+		}
+		catch (Exception expected) {
+		}
+		finally {
+			assertEquals( TransactionStatus.MARKED_ROLLBACK, transactionCoordinator.getTransactionDriverControl().getStatus() );
+			transactionCoordinator.getTransactionDriverControl().rollback();
+		}
+	}
 }