From e53ff5838184aae1159727feff5135b9f4dced0a Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Tue, 8 Oct 2024 17:24:44 +0200 Subject: [PATCH] HHH-18692 Hibernate attempts to close batched statements multiple times --- .../engine/jdbc/batch/internal/BatchImpl.java | 17 +----- .../jdbc/internal/JdbcCoordinatorImpl.java | 15 ++++- .../group/PreparedStatementDetails.java | 4 ++ .../AbstractPreparedStatementGroup.java | 58 +++++++++++++++++++ .../PreparedStatementDetailsStandard.java | 9 +++ .../PreparedStatementGroupSingleTable.java | 8 +-- .../PreparedStatementGroupStandard.java | 11 ++-- 7 files changed, 93 insertions(+), 29 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractPreparedStatementGroup.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/BatchImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/BatchImpl.java index 86b59d2a6f..15c9ca7a81 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/BatchImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/BatchImpl.java @@ -162,26 +162,11 @@ public class BatchImpl implements Batch { if ( batchPosition == batchSizeToUse ) { notifyObserversImplicitExecution(); performExecution(); - batchPosition = 0; - batchExecuted = true; } } protected void releaseStatements() { - statementGroup.forEachStatement( (tableName, statementDetails) -> { - if ( statementDetails.getStatement() == null ) { - BATCH_LOGGER.debugf( - "PreparedStatementDetails did not contain PreparedStatement on #releaseStatements : %s", - statementDetails.getSqlString() - ); - } - else { - clearBatch( statementDetails ); - } - } ); - statementGroup.release(); - jdbcCoordinator.afterStatementExecution(); } protected void clearBatch(PreparedStatementDetails statementDetails) { @@ -299,8 +284,10 @@ public class BatchImpl implements Batch { } } } ); + batchExecuted = true; } finally { + jdbcCoordinator.afterStatementExecution(); batchPosition = 0; } } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java index 3257df2013..08eefae093 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcCoordinatorImpl.java @@ -165,8 +165,12 @@ public class JdbcCoordinatorImpl implements JdbcCoordinator { return currentBatch; } else { - currentBatch.execute(); - currentBatch.release(); + try { + currentBatch.execute(); + } + finally { + currentBatch.release(); + } } } @@ -192,7 +196,12 @@ public class JdbcCoordinatorImpl implements JdbcCoordinator { public void conditionallyExecuteBatch(BatchKey key) { if ( currentBatch != null && !currentBatch.getKey().equals( key ) ) { JdbcBatchLogging.BATCH_LOGGER.debugf( "Conditionally executing batch - %s", currentBatch.getKey() ); - currentBatch.execute(); + try { + currentBatch.execute(); + } + finally { + currentBatch.release(); + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/group/PreparedStatementDetails.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/group/PreparedStatementDetails.java index a924d90c03..61c6497e6a 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/group/PreparedStatementDetails.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/group/PreparedStatementDetails.java @@ -56,4 +56,8 @@ public interface PreparedStatementDetails { } void releaseStatement(SharedSessionContractImplementor session); + + default boolean toRelease(){ + return false; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractPreparedStatementGroup.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractPreparedStatementGroup.java new file mode 100644 index 0000000000..ec65e03a4d --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/AbstractPreparedStatementGroup.java @@ -0,0 +1,58 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.engine.jdbc.mutation.internal; + +import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails; +import org.hibernate.engine.jdbc.mutation.group.PreparedStatementGroup; +import org.hibernate.engine.spi.SharedSessionContractImplementor; + +import java.sql.PreparedStatement; +import java.sql.SQLException; + +import static org.hibernate.engine.jdbc.batch.JdbcBatchLogging.BATCH_LOGGER; +import static org.hibernate.engine.jdbc.batch.JdbcBatchLogging.BATCH_MESSAGE_LOGGER; + +public abstract class AbstractPreparedStatementGroup implements PreparedStatementGroup { + private final SharedSessionContractImplementor session; + + public AbstractPreparedStatementGroup(SharedSessionContractImplementor session) { + this.session = session; + } + + protected void clearBatch(PreparedStatementDetails statementDetails) { + final PreparedStatement statement = statementDetails.getStatement(); + assert statement != null; + + try { + // This code can be called after the connection is released + // and the statement is closed. If the statement is closed, + // then SQLException will be thrown when PreparedStatement#clearBatch + // is called. + // Ensure the statement is not closed before + // calling PreparedStatement#clearBatch. + if ( !statement.isClosed() ) { + statement.clearBatch(); + } + } + catch ( SQLException e ) { + BATCH_MESSAGE_LOGGER.unableToReleaseBatchStatement(); + } + } + + protected void release(PreparedStatementDetails statementDetails) { + if ( statementDetails.toRelease() ) { + if ( statementDetails.getStatement() == null ) { + BATCH_LOGGER.debugf( + "PreparedStatementDetails did not contain PreparedStatement on #releaseStatements : %s", + statementDetails.getSqlString() + ); + } + else { + clearBatch( statementDetails ); + } + statementDetails.releaseStatement( session ); + } + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementDetailsStandard.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementDetailsStandard.java index 9be6d7cd47..478c40fb98 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementDetailsStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementDetailsStandard.java @@ -30,6 +30,8 @@ public class PreparedStatementDetailsStandard implements PreparedStatementDetail private PreparedStatement statement; + private boolean toRelease; + public PreparedStatementDetailsStandard( PreparableMutationOperation tableMutation, Supplier jdbcStatementCreator, @@ -66,6 +68,7 @@ public class PreparedStatementDetailsStandard implements PreparedStatementDetail if ( statement != null ) { session.getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( statement ); statement = null; + toRelease = false; } } @@ -82,6 +85,7 @@ public class PreparedStatementDetailsStandard implements PreparedStatementDetail @Override public PreparedStatement resolveStatement() { if ( statement == null ) { + toRelease = true; statement = jdbcStatementCreator.get(); try { expectation.prepare( statement ); @@ -102,6 +106,11 @@ public class PreparedStatementDetailsStandard implements PreparedStatementDetail return expectation; } + @Override + public boolean toRelease() { + return toRelease; + } + @Override public String toString() { return "PreparedStatementDetails(" + sql + ")"; diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupSingleTable.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupSingleTable.java index 00cac1c56d..bcbc765818 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupSingleTable.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupSingleTable.java @@ -8,7 +8,6 @@ import java.util.function.BiConsumer; import java.util.function.Predicate; import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails; -import org.hibernate.engine.jdbc.mutation.group.PreparedStatementGroup; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.generator.values.GeneratedValuesMutationDelegate; import org.hibernate.sql.model.PreparableMutationOperation; @@ -20,9 +19,8 @@ import org.hibernate.sql.model.TableMapping; * * @author Steve Ebersole */ -public class PreparedStatementGroupSingleTable implements PreparedStatementGroup { +public class PreparedStatementGroupSingleTable extends AbstractPreparedStatementGroup { private final PreparableMutationOperation jdbcMutation; - private final SharedSessionContractImplementor session; private final PreparedStatementDetails statementDetails; @@ -36,9 +34,9 @@ public class PreparedStatementGroupSingleTable implements PreparedStatementGroup PreparableMutationOperation jdbcMutation, GeneratedValuesMutationDelegate delegate, SharedSessionContractImplementor session) { + super(session); this.jdbcMutation = jdbcMutation; this.statementDetails = ModelMutationHelper.standardPreparation( jdbcMutation, delegate, session ); - this.session = session; } protected TableMapping getMutatingTableDetails() { @@ -89,7 +87,7 @@ public class PreparedStatementGroupSingleTable implements PreparedStatementGroup @Override public void release() { if ( statementDetails != null ) { - statementDetails.releaseStatement( session ); + release( statementDetails ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupStandard.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupStandard.java index 820b72f989..5a74624e84 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/mutation/internal/PreparedStatementGroupStandard.java @@ -16,7 +16,6 @@ import java.util.function.Predicate; import java.util.function.Supplier; import org.hibernate.engine.jdbc.mutation.group.PreparedStatementDetails; -import org.hibernate.engine.jdbc.mutation.group.PreparedStatementGroup; import org.hibernate.engine.jdbc.spi.JdbcCoordinator; import org.hibernate.engine.jdbc.spi.MutationStatementPreparer; import org.hibernate.engine.spi.SharedSessionContractImplementor; @@ -32,11 +31,10 @@ import org.hibernate.sql.model.TableMapping; * * @author Steve Ebersole */ -public class PreparedStatementGroupStandard implements PreparedStatementGroup { +public class PreparedStatementGroupStandard extends AbstractPreparedStatementGroup { private final MutationType mutationType; private final MutationTarget mutationTarget; private final List jdbcMutations; - private final SharedSessionContractImplementor session; private final SortedMap statementMap; @@ -47,11 +45,11 @@ public class PreparedStatementGroupStandard implements PreparedStatementGroup { GeneratedValuesMutationDelegate generatedValuesDelegate, List jdbcMutations, SharedSessionContractImplementor session) { + super( session ); this.mutationType = mutationType; this.mutationTarget = mutationTarget; this.jdbcMutations = jdbcMutations; - this.session = session; this.statementMap = createStatementDetailsMap( jdbcMutations, mutationType, generatedValuesDelegate, session ); } @@ -143,10 +141,11 @@ public class PreparedStatementGroupStandard implements PreparedStatementGroup { @Override public void release() { - statementMap.forEach( (tableName, statementDetails) -> statementDetails.releaseStatement( session ) ); + statementMap.forEach( (tableName, statementDetails) -> { + release( statementDetails ); + } ); } - private static SortedMap createStatementDetailsMap( List jdbcMutations, MutationType mutationType,