From fa6a222ab1dd9925d95f211f38c951e0db4095ad Mon Sep 17 00:00:00 2001 From: Scott Marlow Date: Tue, 1 Oct 2013 16:05:09 -0400 Subject: [PATCH] HHH-8586 Synchronization beforeCompletion/afterCompletion should check if EM is closed and this helps us pass the tck entityManagerFactoryCloseExceptions test. --- .../internal/TransactionCoordinatorImpl.java | 5 + .../spi/TransactionCoordinator.java | 2 + ...ynchronizationCallbackCoordinatorImpl.java | 7 ++ .../internal/AbstractSessionImpl.java | 4 +- .../test/EntityManagerFactoryClosedTest.java | 100 ++++++++++++++++++ 5 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/EntityManagerFactoryClosedTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionCoordinatorImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionCoordinatorImpl.java index 5d4f2fdacf..82c6a357c4 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionCoordinatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionCoordinatorImpl.java @@ -346,6 +346,11 @@ public class TransactionCoordinatorImpl implements TransactionCoordinator { synchronizationRegistry.notifySynchronizationsAfterTransactionCompletion( status ); } + @Override + public boolean isActive() { + return !sessionFactory().isClosed(); + } + // serialization ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/hibernate-core/src/main/java/org/hibernate/engine/transaction/spi/TransactionCoordinator.java b/hibernate-core/src/main/java/org/hibernate/engine/transaction/spi/TransactionCoordinator.java index 52f9054be0..0b06aff160 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/transaction/spi/TransactionCoordinator.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/transaction/spi/TransactionCoordinator.java @@ -141,4 +141,6 @@ public interface TransactionCoordinator extends Serializable { public void sendBeforeTransactionCompletionNotifications(TransactionImplementor hibernateTransaction); public void sendAfterTransactionCompletionNotifications(TransactionImplementor hibernateTransaction, int status); + public boolean isActive(); + } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/transaction/synchronization/internal/SynchronizationCallbackCoordinatorImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/transaction/synchronization/internal/SynchronizationCallbackCoordinatorImpl.java index 3e5d40e0c3..2141da632e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/transaction/synchronization/internal/SynchronizationCallbackCoordinatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/transaction/synchronization/internal/SynchronizationCallbackCoordinatorImpl.java @@ -95,6 +95,10 @@ public class SynchronizationCallbackCoordinatorImpl implements SynchronizationCa public void beforeCompletion() { LOG.trace( "Transaction before completion callback" ); + if ( !transactionCoordinator.isActive() ) { + return; + } + boolean flush; try { final int status = transactionCoordinator.getTransactionContext().getTransactionEnvironment() @@ -160,6 +164,9 @@ public class SynchronizationCallbackCoordinatorImpl implements SynchronizationCa private void doAfterCompletion(int status) { LOG.tracev( "Transaction afterCompletion callback [status={0}]", status ); + if ( !transactionCoordinator.isActive() ) { + return; + } try { afterCompletionAction.doAction( transactionCoordinator, status ); diff --git a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSessionImpl.java index 4252a21824..f8d8f0136f 100755 --- a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSessionImpl.java @@ -120,7 +120,7 @@ public abstract class AbstractSessionImpl @Override public boolean isClosed() { - return closed; + return closed || factory.isClosed(); } protected void setClosed() { @@ -128,7 +128,7 @@ public abstract class AbstractSessionImpl } protected void errorIfClosed() { - if ( closed ) { + if ( isClosed() ) { throw new SessionException( "Session is closed!" ); } } diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/EntityManagerFactoryClosedTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/EntityManagerFactoryClosedTest.java new file mode 100644 index 0000000000..cd6a12cba4 --- /dev/null +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/EntityManagerFactoryClosedTest.java @@ -0,0 +1,100 @@ +//$Id$ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2010, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.jpa.test; + +import org.hibernate.engine.transaction.internal.jta.JtaStatusHelper; +import org.hibernate.jpa.AvailableSettings; +import org.hibernate.testing.jta.TestingJtaBootstrap; +import org.hibernate.testing.jta.TestingJtaPlatformImpl; +import org.junit.Test; + + +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; + +import java.util.Map; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + + +/** + * EntityManagerFactoryClosedTest + * + * @author Scott Marlow + */ +public class EntityManagerFactoryClosedTest extends BaseEntityManagerFunctionalTestCase { + + @Override + protected void addConfigOptions(Map options) { + super.addConfigOptions( options ); + TestingJtaBootstrap.prepare(options); + options.put( AvailableSettings.TRANSACTION_TYPE, "JTA" ); + } + + /** + * Test that using a closed EntityManagerFactory throws an IllegalStateException + * Also ensure that HHH-8586 doesn't regress. + * @throws Exception + */ + @Test + public void testWithTransactionalEnvironment() throws Exception { + + assertFalse( JtaStatusHelper.isActive(TestingJtaPlatformImpl.INSTANCE.getTransactionManager()) ); + TestingJtaPlatformImpl.INSTANCE.getTransactionManager().begin(); + assertTrue( JtaStatusHelper.isActive(TestingJtaPlatformImpl.INSTANCE.getTransactionManager()) ); + EntityManagerFactory entityManagerFactory = entityManagerFactory(); + + entityManagerFactory.close(); // close the underlying entity manager factory + + try { + entityManagerFactory.createEntityManager(); + fail( "expected IllegalStateException when calling emf.createEntityManager with closed emf" ); + } catch( IllegalStateException expected ) { + // success + + } + + try { + entityManagerFactory.getCriteriaBuilder(); + fail( "expected IllegalStateException when calling emf.getCriteriaBuilder with closed emf" ); + } catch( IllegalStateException expected ) { + // success + } + + try { + entityManagerFactory.getCache(); + fail( "expected IllegalStateException when calling emf.getCache with closed emf" ); + } catch( IllegalStateException expected ) { + // success + } + + assertFalse( entityManagerFactory.isOpen() ); + + TestingJtaPlatformImpl.INSTANCE.getTransactionManager().commit(); + } + +}