HHH-18692 Hibernate attempts to close batched statements multiple times

dreab8 · dreab8 · commit e53ff5838184 · 2024-12-04T14:33:54.000+01:00
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
@@ -162,26 +162,11 @@ public void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecke
 		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 @@ protected void performExecution() {
 					}
 				}
 			} );
+			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
@@ -165,8 +165,12 @@ public Batch getBatch(BatchKey key, Integer batchSize, Supplier<PreparedStatemen
 				return currentBatch;
 			}
 			else {
-				currentBatch.execute();
-				currentBatch.release();
+				try {
+					currentBatch.execute();
+				}
+				finally {
+					currentBatch.release();
+				}
 			}
 		}
 
@@ -192,7 +196,12 @@ public void executeBatch() {
 	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
@@ -56,4 +56,8 @@ default boolean isCallable() {
 	}
 
 	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
@@ -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
@@ -30,6 +30,8 @@ public class PreparedStatementDetailsStandard implements PreparedStatementDetail
 
 	private PreparedStatement statement;
 
+	private boolean toRelease;
+
 	public PreparedStatementDetailsStandard(
 			PreparableMutationOperation tableMutation,
 			Supplier<PreparedStatement> jdbcStatementCreator,
@@ -66,6 +68,7 @@ public void releaseStatement(SharedSessionContractImplementor session) {
 		if ( statement != null ) {
 			session.getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( statement );
 			statement = null;
+			toRelease = false;
 		}
 	}
 
@@ -82,6 +85,7 @@ public PreparedStatement getStatement() {
 	@Override
 	public PreparedStatement resolveStatement() {
 		if ( statement == null ) {
+			toRelease = true;
 			statement = jdbcStatementCreator.get();
 			try {
 				expectation.prepare( statement );
@@ -102,6 +106,11 @@ public Expectation getExpectation() {
 		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
@@ -8,7 +8,6 @@
 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 @@
  *
  * @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 PreparedStatementGroupSingleTable(
 			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 boolean hasMatching(Predicate<PreparedStatementDetails> filter) {
 	@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
@@ -16,7 +16,6 @@
 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 @@
  *
  * @author Steve Ebersole
  */
-public class PreparedStatementGroupStandard implements PreparedStatementGroup {
+public class PreparedStatementGroupStandard extends AbstractPreparedStatementGroup {
 	private final MutationType mutationType;
 	private final MutationTarget<?> mutationTarget;
 	private final List<PreparableMutationOperation> jdbcMutations;
-	private final SharedSessionContractImplementor session;
 
 	private final SortedMap<String, PreparedStatementDetails> statementMap;
 
@@ -47,11 +45,11 @@ public PreparedStatementGroupStandard(
 			GeneratedValuesMutationDelegate generatedValuesDelegate,
 			List<PreparableMutationOperation> 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 @@ private static PreparedStatementDetails createPreparedStatementDetails(
 
 	@Override
 	public void release() {
-		statementMap.forEach( (tableName, statementDetails) -> statementDetails.releaseStatement( session ) );
+		statementMap.forEach( (tableName, statementDetails) -> {
+			release( statementDetails );
+		} );
 	}
 
-
 	private static SortedMap<String, PreparedStatementDetails> createStatementDetailsMap(
 			List<PreparableMutationOperation> jdbcMutations,
 			MutationType mutationType,

Original file line number	Diff line number	Diff line change
`@@ -162,26 +162,11 @@ public void addToBatch(JdbcValueBindings jdbcValueBindings, TableInclusionChecke`
`162`	`162`	`if ( batchPosition == batchSizeToUse ) {`
`163`	`163`	`notifyObserversImplicitExecution();`
`164`	`164`	`performExecution();`
`165`		`- batchPosition = 0;`
`166`		`- batchExecuted = true;`
`167`	`165`	`}`
`168`	`166`	`}`
`169`	`167`
`170`	`168`	`protected void releaseStatements() {`
`171`		`- statementGroup.forEachStatement( (tableName, statementDetails) -> {`
`172`		`- if ( statementDetails.getStatement() == null ) {`
`173`		`- BATCH_LOGGER.debugf(`
`174`		`- "PreparedStatementDetails did not contain PreparedStatement on #releaseStatements : %s",`
`175`		`- statementDetails.getSqlString()`
`176`		`- );`
`177`		`- }`
`178`		`- else {`
`179`		`- clearBatch( statementDetails );`
`180`		`- }`
`181`		`- } );`
`182`		`-`
`183`	`169`	`statementGroup.release();`
`184`		`- jdbcCoordinator.afterStatementExecution();`
`185`	`170`	`}`
`186`	`171`
`187`	`172`	`protected void clearBatch(PreparedStatementDetails statementDetails) {`
`@@ -299,8 +284,10 @@ protected void performExecution() {`
`299`	`284`	`}`
`300`	`285`	`}`
`301`	`286`	`} );`
	`287`	`+ batchExecuted = true;`
`302`	`288`	`}`
`303`	`289`	`finally {`
	`290`	`+ jdbcCoordinator.afterStatementExecution();`
`304`	`291`	`batchPosition = 0;`
`305`	`292`	`}`
`306`	`293`	`}`
Original file line number	Diff line number	Diff line change
`@@ -165,8 +165,12 @@ public Batch getBatch(BatchKey key, Integer batchSize, Supplier<PreparedStatemen`
`165`	`165`	`return currentBatch;`
`166`	`166`	`}`
`167`	`167`	`else {`
`168`		`- currentBatch.execute();`
`169`		`- currentBatch.release();`
	`168`	`+ try {`
	`169`	`+ currentBatch.execute();`
	`170`	`+ }`
	`171`	`+ finally {`
	`172`	`+ currentBatch.release();`
	`173`	`+ }`
`170`	`174`	`}`
`171`	`175`	`}`
`172`	`176`
`@@ -192,7 +196,12 @@ public void executeBatch() {`
`192`	`196`	`public void conditionallyExecuteBatch(BatchKey key) {`
`193`	`197`	`if ( currentBatch != null && !currentBatch.getKey().equals( key ) ) {`
`194`	`198`	`JdbcBatchLogging.BATCH_LOGGER.debugf( "Conditionally executing batch - %s", currentBatch.getKey() );`
`195`		`- currentBatch.execute();`
	`199`	`+ try {`
	`200`	`+ currentBatch.execute();`
	`201`	`+ }`
	`202`	`+ finally {`
	`203`	`+ currentBatch.release();`
	`204`	`+ }`
`196`	`205`	`}`
`197`	`206`	`}`
`198`	`207`
Original file line number	Diff line number	Diff line change
`@@ -56,4 +56,8 @@ default boolean isCallable() {`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`void releaseStatement(SharedSessionContractImplementor session);`
	`59`	`+`
	`60`	`+ default boolean toRelease(){`
	`61`	`+ return false;`
	`62`	`+ }`
`59`	`63`	`}`