From 9a9f027f824d1838935e0382c8778098f96aad71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 12 Apr 2023 09:53:22 +0200 Subject: [PATCH] HHH-16458 Close JDBC statement when DeferredResultSetAccess fails to execute a query --- .../internal/DeferredResultSetAccess.java | 6 + .../orm/test/query/QuerySqlExceptionTest.java | 105 ++++++++++++++++++ ...reparedStatementSpyConnectionProvider.java | 8 ++ 3 files changed, 119 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/query/QuerySqlExceptionTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java index ff84756287..afb80ebf9f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java @@ -249,6 +249,12 @@ public class DeferredResultSetAccess extends AbstractResultSetAccess { } catch (SQLException e) { + try { + release(); + } + catch (RuntimeException e2) { + e.addSuppressed( e2 ); + } throw executionContext.getSession().getJdbcServices().getSqlExceptionHelper().convert( e, "JDBC exception executing SQL [" + finalSql + "]" diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/QuerySqlExceptionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/QuerySqlExceptionTest.java new file mode 100644 index 0000000000..c070f730d5 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/QuerySqlExceptionTest.java @@ -0,0 +1,105 @@ +/* + * 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 http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.query; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.sql.SQLException; +import java.time.LocalDate; +import java.util.List; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.dialect.MySQLDialect; +import org.hibernate.engine.spi.SessionFactoryImplementor; + +import org.hibernate.testing.orm.domain.StandardDomainModel; +import org.hibernate.testing.orm.domain.contacts.Contact; +import org.hibernate.testing.orm.jdbc.PreparedStatementSpyConnectionProvider; +import org.hibernate.testing.orm.jdbc.PreparedStatementSpyConnectionProviderSettingProvider; +import org.hibernate.testing.orm.junit.DialectFeatureChecks; +import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.Jpa; +import org.hibernate.testing.orm.junit.RequiresDialectFeature; +import org.hibernate.testing.orm.junit.SettingProvider; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.Query; + +@Jpa( + standardModels = StandardDomainModel.CONTACTS, + settingProviders = { + @SettingProvider(settingName = AvailableSettings.CONNECTION_PROVIDER, + provider = PreparedStatementSpyConnectionProviderSettingProvider.class) + } +) +@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsJdbcDriverProxying.class) +public class QuerySqlExceptionTest { + + private PreparedStatementSpyConnectionProvider connectionProvider; + + @BeforeAll + public void init(EntityManagerFactoryScope scope) { + connectionProvider = (PreparedStatementSpyConnectionProvider) scope.getEntityManagerFactory().getProperties() + .get( AvailableSettings.CONNECTION_PROVIDER ); + } + + @Test + public void sqlExceptionOnExecutionWillCloseStatement(EntityManagerFactoryScope scope) { + // We need at least one row in the "contacts" table, + // otherwise the SELECT below might not even get executed completely + // (the DB somehow detects the result will be 0 rows anyway and doesn't bother evaluating parameters). + scope.inTransaction( entityManager -> { + var contact = new Contact( + 1, + new Contact.Name( "John", "Doe" ), + Contact.Gender.MALE, + LocalDate.of( 1970, 1, 1 ) + ); + entityManager.persist( contact ); + } ); + scope.inTransaction( entityManager -> { + connectionProvider.clear(); + assertThatThrownBy( () -> createQueryTriggeringStatementExecutionFailure( entityManager ).getResultList() ) + .satisfiesAnyOf( + // Behavior differs depending on the dialect + e -> assertThat( e ).isInstanceOf( SQLException.class ), + e -> assertThat( e ).hasCauseInstanceOf( SQLException.class ), + e -> assertThat( e ).hasRootCauseInstanceOf( SQLException.class ) + ); + // Checking immediately, because the JDBC driver or connection pool might "fix" statement leaks + // when the connection gets closed on transaction commit. + assertThat( connectionProvider.getPreparedStatementsAndSql().entrySet() ) + .isNotEmpty() + .allSatisfy( e -> assertThat( e.getKey().isClosed() ) + .as( "Statement '" + e.getValue() + "' is closed" ) + .isTrue() ); + } ); + } + + // Creates a query that will intentionally trigger an exception + // during statement execution (not during preparation). + private Query createQueryTriggeringStatementExecutionFailure(EntityManager entityManager) { + var dialect = entityManager.getEntityManagerFactory().unwrap( SessionFactoryImplementor.class ) + .getJdbcServices().getDialect(); + Object badParamValue; + if ( dialect instanceof MySQLDialect ) { + // These databases are perfectly fine with the operation `"foo" / 2` + // and will happily return `0.0` without any error... + // Let's give them something even more nonsensical + // (but which we cannot pass to other DBs as they would detect the problem too early) + badParamValue = List.of( "foo", "bar" ); + } + else { + badParamValue = "foo"; + } + return entityManager.createNativeQuery( "select ( :param / 2 ) from contacts" ) + .setParameter( "param", badParamValue ); + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/jdbc/PreparedStatementSpyConnectionProvider.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/jdbc/PreparedStatementSpyConnectionProvider.java index d611e15914..3fd9e4a352 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/jdbc/PreparedStatementSpyConnectionProvider.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/jdbc/PreparedStatementSpyConnectionProvider.java @@ -150,6 +150,14 @@ public class PreparedStatementSpyConnectionProvider extends ConnectionProviderDe return new ArrayList<>( preparedStatementMap.keySet() ); } + /** + * @return the PreparedStatements that were executed since the last clear operation, + * along with each statement's corresponding SQL. + */ + public Map getPreparedStatementsAndSql() { + return preparedStatementMap; + } + /** * Get the PreparedStatements SQL statements. *