From eef8a48ce472a76d088f6450ae6c0e7adef671a8 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Thu, 6 Apr 2017 18:37:00 +0100 Subject: [PATCH] HHH-11617 - Statement leak in case of 'SQLGrammarException: could not extract ResultSet' --- .../internal/AbstractLoadPlanBasedLoader.java | 4 +- ...tementIsClosedAfterALockExceptionTest.java | 121 ++++++++++++++++++ .../testing/transaction/TransactionUtil.java | 27 +++- 3 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/jpa/test/lock/StatementIsClosedAfterALockExceptionTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractLoadPlanBasedLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractLoadPlanBasedLoader.java index c13577754e..e8f6cd0680 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractLoadPlanBasedLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/AbstractLoadPlanBasedLoader.java @@ -443,10 +443,10 @@ protected final ResultSet getResultSet( } return rs; } - catch ( SQLException sqle ) { + catch (SQLException | HibernateException ex) { session.getJdbcCoordinator().getResourceRegistry().release( st ); session.getJdbcCoordinator().afterStatementExecution(); - throw sqle; + throw ex; } } diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/test/lock/StatementIsClosedAfterALockExceptionTest.java b/hibernate-core/src/test/java/org/hibernate/jpa/test/lock/StatementIsClosedAfterALockExceptionTest.java new file mode 100644 index 0000000000..8f64477757 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/jpa/test/lock/StatementIsClosedAfterALockExceptionTest.java @@ -0,0 +1,121 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.jpa.test.lock; + +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import javax.persistence.LockModeType; + +import org.hibernate.Session; +import org.hibernate.jpa.AvailableSettings; +import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase; + +import org.hibernate.testing.DialectChecks; +import org.hibernate.testing.RequiresDialectFeature; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.jdbc.PreparedStatementSpyConnectionProvider; +import org.hibernate.testing.transaction.TransactionUtil; +import org.hibernate.testing.util.ExceptionUtil; +import org.junit.Before; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +/** + * @author Andrea Boriero + */ +public class StatementIsClosedAfterALockExceptionTest extends BaseEntityManagerFunctionalTestCase { + + private static final PreparedStatementSpyConnectionProvider CONNECTION_PROVIDER = new PreparedStatementSpyConnectionProvider(); + + private Integer lockId; + + @Override + protected Map getConfig() { + Map config = super.getConfig(); + config.put( + org.hibernate.cfg.AvailableSettings.CONNECTION_PROVIDER, + CONNECTION_PROVIDER + ); + return config; + } + + @Before + public void setUp() { + lockId = TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + Lock lock = new Lock(); + lock.setName( "name" ); + entityManager.persist( lock ); + return lock.getId(); + } ); + } + + @Override + public void releaseResources() { + super.releaseResources(); + CONNECTION_PROVIDER.stop(); + } + + @Test(timeout = 2500) //2.5 seconds + @TestForIssue(jiraKey = "HHH-11617") + public void testStatementIsClosed() { + + TransactionUtil.doInJPA( this::entityManagerFactory, em1 -> { + + Map properties = new HashMap<>(); + properties.put( org.hibernate.cfg.AvailableSettings.JPA_LOCK_TIMEOUT, 0L ); + Lock lock2 = em1.find( Lock.class, lockId, LockModeType.PESSIMISTIC_WRITE, properties ); + assertEquals( + "lock mode should be PESSIMISTIC_WRITE ", + LockModeType.PESSIMISTIC_WRITE, + em1.getLockMode( lock2 ) + ); + + TransactionUtil.doInJPA( this::entityManagerFactory, em2 -> { + TransactionUtil.setJdbcTimeout( em2.unwrap( Session.class ) ); + try { + em2.find( Lock.class, lockId, LockModeType.PESSIMISTIC_WRITE, properties ); + fail( "Exception should be thrown" ); + } + catch (Exception lte) { + if( !ExceptionUtil.isSqlLockTimeout( lte )) { + fail("Should have thrown a Lock timeout exception"); + } + } + finally { + try { + for ( PreparedStatement statement : CONNECTION_PROVIDER.getPreparedStatements() ) { + assertThat( + "A SQL Statement was not closed : " + statement.toString(), + statement.isClosed(), + is( true ) + ); + } + } + catch (SQLException e) { + fail( e.getMessage() ); + } + } + } ); + + } ); + } + + @Override + public Class[] getAnnotatedClasses() { + return new Class[] { + Lock.class, + UnversionedLock.class + }; + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/transaction/TransactionUtil.java b/hibernate-testing/src/main/java/org/hibernate/testing/transaction/TransactionUtil.java index 6373222f88..257d9edaf0 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/transaction/TransactionUtil.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/transaction/TransactionUtil.java @@ -6,10 +6,12 @@ */ package org.hibernate.testing.transaction; +import java.sql.PreparedStatement; import java.sql.Statement; import java.util.Map; import java.util.Properties; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -445,32 +447,43 @@ public static void doInHibernateSessionBuilder( } } } - /** * Set Session or Statement timeout * @param session Hibernate Session */ public static void setJdbcTimeout(Session session) { + setJdbcTimeout( session, TimeUnit.SECONDS.toMillis( 1 ) ); + } + + /** + * Set Session or Statement timeout + * @param session Hibernate Session + */ + public static void setJdbcTimeout(Session session, long millis) { + session.doWork( connection -> { if ( Dialect.getDialect() instanceof PostgreSQL81Dialect ) { try (Statement st = connection.createStatement()) { - st.execute( "SET statement_timeout TO 1000" ); + //Prepared Statements fail for SET commands + st.execute(String.format( "SET statement_timeout TO %d", millis / 10)); } } else if( Dialect.getDialect() instanceof MySQLDialect ) { - try (Statement st = connection.createStatement()) { - st.execute( "SET GLOBAL innodb_lock_wait_timeout = 1" ); + try (PreparedStatement st = connection.prepareStatement("SET GLOBAL innodb_lock_wait_timeout = ?")) { + st.setLong( 1, TimeUnit.MILLISECONDS.toSeconds( millis ) ); + st.execute(); } } else if( Dialect.getDialect() instanceof H2Dialect ) { - try (Statement st = connection.createStatement()) { - st.execute( "SET LOCK_TIMEOUT 100" ); + try (PreparedStatement st = connection.prepareStatement("SET LOCK_TIMEOUT ?")) { + st.setLong( 1, millis / 10 ); + st.execute(); } } else { try { - connection.setNetworkTimeout( Executors.newSingleThreadExecutor(), 1000 ); + connection.setNetworkTimeout( Executors.newSingleThreadExecutor(), (int) millis ); } catch (Throwable ignore) { ignore.fillInStackTrace();