HHH-13050 (5.3) On release of batch it still contained JDBC statements logged; unable to release batch statement

hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/AbstractBatchImpl.java

@@ -40,8 +40,8 @@ public abstract class AbstractBatchImpl implements Batch {
 	private final SqlStatementLogger sqlStatementLogger;
 	private final SqlExceptionHelper sqlExceptionHelper;
 
-	private LinkedHashMap<String,PreparedStatement> statements = new LinkedHashMap<String,PreparedStatement>();
-	private LinkedHashSet<BatchObserver> observers = new LinkedHashSet<BatchObserver>();
+	private LinkedHashMap<String, PreparedStatement> statements = new LinkedHashMap<>();
+	private LinkedHashSet<BatchObserver> observers = new LinkedHashSet<>();
 
 	protected AbstractBatchImpl(BatchKey key, JdbcCoordinator jdbcCoordinator) {
 		if ( key == null ) {
@@ -162,7 +162,15 @@ protected void releaseStatements() {
 
 	protected void clearBatch(PreparedStatement statement) {
 		try {
-			statement.clearBatch();
+			// 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 ) {
 			LOG.unableToReleaseBatchStatement();

hibernate-core/src/main/java/org/hibernate/engine/jdbc/batch/internal/BatchingBatch.java

@@ -77,6 +77,7 @@ public void addToBatch() {
 			currentStatement.addBatch();
 		}
 		catch ( SQLException e ) {
+			abortBatch();
 			LOG.debugf( "SQLException escaped proxy", e );
 			throw sqlExceptionHelper().convert( e, "could not perform addBatch", currentStatementSql );
 		}

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -29,6 +29,7 @@
 import org.hibernate.EntityMode;
 import org.hibernate.FetchMode;
 import org.hibernate.HibernateException;
+import org.hibernate.JDBCException;
 import org.hibernate.LockMode;
 import org.hibernate.LockOptions;
 import org.hibernate.MappingException;
@@ -3171,9 +3172,8 @@ protected void insert(
 									.executeUpdate( insert ), insert, -1
 					);
 				}
-
 			}
-			catch (SQLException e) {
+			catch (SQLException | JDBCException e) {
 				if ( useBatch ) {
 					session.getJdbcCoordinator().abortBatch();
 				}

hibernate-core/src/test/java/org/hibernate/jpa/test/transaction/batch/AbstractJtaBatchTest.java

@@ -0,0 +1,140 @@
+/*
+ * 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.jpa.test.transaction.batch;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.util.List;
+import java.util.Map;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.Id;
+
+import org.hibernate.annotations.GenericGenerator;
+import org.hibernate.annotations.Parameter;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.engine.jdbc.batch.internal.AbstractBatchImpl;
+import org.hibernate.engine.jdbc.batch.internal.BatchBuilderInitiator;
+import org.hibernate.internal.CoreMessageLogger;
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+
+import org.hibernate.testing.jta.TestingJtaBootstrap;
+import org.hibernate.testing.logger.LoggerInspectionRule;
+import org.hibernate.testing.logger.Triggerable;
+import org.junit.Before;
+import org.junit.Rule;
+
+import org.jboss.logging.Logger;
+
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Andrea Boriero
+ */
+public abstract class AbstractJtaBatchTest extends BaseEntityManagerFunctionalTestCase {
+
+	@Rule
+	public LoggerInspectionRule logInspection = new LoggerInspectionRule(
+			Logger.getMessageLogger( CoreMessageLogger.class, AbstractBatchImpl.class.getName() )
+	);
+
+	protected Triggerable triggerable;
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] { Comment.class, EventLog.class };
+	}
+
+	@Override
+	protected void addConfigOptions(Map options) {
+		super.addConfigOptions( options );
+		TestingJtaBootstrap.prepare( options );
+		options.put( BatchBuilderInitiator.BUILDER, getBatchBuilderClassName() );
+		options.put( AvailableSettings.JPA_TRANSACTION_COMPLIANCE, "true" );
+		options.put( AvailableSettings.JPA_TRANSACTION_TYPE, "JTA" );
+		options.put( AvailableSettings.STATEMENT_BATCH_SIZE, "50" );
+	}
+
+	@Before
+	public void setUp() {
+		triggerable = logInspection.watchForLogMessages(
+				"HHH000352: Unable to release batch statement..." );
+		triggerable.reset();
+	}
+
+	protected void assertAllStatementsAreClosed(List<PreparedStatement> statements) {
+		statements.forEach( statement -> {
+			try {
+				assertThat( "A PreparedStatement has not been closed", statement.isClosed(), is( true ) );
+			}
+			catch (SQLException e) {
+				fail( e.getMessage() );
+			}
+		} );
+	}
+
+	protected abstract String getBatchBuilderClassName();
+
+	@Entity(name = "Comment")
+	public static class Comment {
+		private Long id;
+		private String message;
+
+		@Id
+		@GeneratedValue(strategy = GenerationType.IDENTITY)
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long id) {
+			this.id = id;
+		}
+
+		public String getMessage() {
+			return message;
+		}
+
+		public void setMessage(String message) {
+			this.message = message;
+		}
+	}
+
+	@Entity(name = "EventLog")
+	public static class EventLog {
+		private Long id;
+		private String message;
+
+		@Id
+		@GeneratedValue(generator = "eventLogIdGenerator")
+		@GenericGenerator(name = "eventLogIdGenerator", strategy = "org.hibernate.id.enhanced.TableGenerator", parameters = {
+				@Parameter(name = "table_name", value = "primaryKeyPools"),
+				@Parameter(name = "segment_value", value = "eventLog"),
+				@Parameter(name = "optimizer", value = "pooled"),
+				@Parameter(name = "increment_size", value = "500"),
+				@Parameter(name = "initial_value", value = "1")
+		})
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long id) {
+			this.id = id;
+		}
+
+		public String getMessage() {
+			return message;
+		}
+
+		public void setMessage(String message) {
+			this.message = message;
+		}
+	}
+
+}

hibernate-core/src/test/java/org/hibernate/jpa/test/transaction/batch/JtaWithFailingBatchTest.java

@@ -0,0 +1,170 @@
+/*
+ * 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.jpa.test.transaction.batch;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.persistence.EntityManager;
+import javax.persistence.FlushModeType;
+import javax.transaction.Status;
+import javax.transaction.TransactionManager;
+
+import org.hibernate.engine.jdbc.batch.internal.BatchBuilderImpl;
+import org.hibernate.engine.jdbc.batch.internal.BatchingBatch;
+import org.hibernate.engine.jdbc.batch.spi.Batch;
+import org.hibernate.engine.jdbc.batch.spi.BatchKey;
+import org.hibernate.engine.jdbc.spi.JdbcCoordinator;
+
+import org.hibernate.testing.DialectChecks;
+import org.hibernate.testing.RequiresDialectFeature;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.jta.TestingJtaPlatformImpl;
+import org.junit.Test;
+
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+
+/**
+ * @author Gail Badner
+ * @author Andrea Boriero
+ */
+@TestForIssue(jiraKey = "HHH-13050")
+@RequiresDialectFeature(DialectChecks.SupportsIdentityColumns.class)
+public class JtaWithFailingBatchTest extends AbstractJtaBatchTest {
+
+	private static TestBatch testBatch;
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] { Comment.class, EventLog.class };
+	}
+
+	@Test
+	public void testAllStatementsAreClosedInCaseOfBatchExecutionFailure() throws Exception {
+		TransactionManager transactionManager = TestingJtaPlatformImpl.INSTANCE.getTransactionManager();
+		EntityManager em = createEntityManager();
+		try {
+			transactionManager.begin();
+
+			em.setFlushMode( FlushModeType.AUTO );
+
+			// Persist entity with non-generated id
+			EventLog eventLog1 = new EventLog();
+			eventLog1.setMessage( "Foo1" );
+			em.persist( eventLog1 );
+
+			// Persist entity with non-generated id
+			EventLog eventLog2 = new EventLog();
+			eventLog2.setMessage( "Foo2" );
+			em.persist( eventLog2 );
+
+			Comment comment = new Comment();
+			comment.setMessage( "Bar" );
+
+			try {
+				em.persist( comment );
+				transactionManager.commit();
+			}
+			catch (Exception expected) {
+				//expected
+				if ( transactionManager.getStatus() == Status.STATUS_ACTIVE ) {
+					transactionManager.rollback();
+				}
+			}
+
+			assertThat(
+					"AbstractBatchImpl#releaseStatements() has not been callled",
+					testBatch.calledReleaseStatements,
+					is( true )
+			);
+			assertAllStatementsAreClosed( testBatch.createdStatements );
+			assertStatementsListIsCleared();
+		}
+		finally {
+
+			em.close();
+		}
+
+		assertFalse( "HHH000352: Unable to release batch statement... has been thrown", triggerable.wasTriggered() );
+	}
+
+	private void assertStatementsListIsCleared() {
+		assertThat( testBatch.createdStatements.size(), not( 0 ) );
+		assertThat(
+				"Not all PreparedStatements have been released",
+				testBatch.numberOfStatementsAfterReleasing,
+				is( 0 )
+		);
+	}
+
+	public static class TestBatch extends BatchingBatch {
+		private int numberOfStatementsAfterReleasing;
+		private List<PreparedStatement> createdStatements = new ArrayList<>();
+		private boolean calledReleaseStatements;
+
+		private String currentStatementSql;
+
+		public TestBatch(BatchKey key, JdbcCoordinator jdbcCoordinator, int batchSize) {
+			super( key, jdbcCoordinator, batchSize );
+		}
+
+		@Override
+		public PreparedStatement getBatchStatement(String sql, boolean callable) {
+			currentStatementSql = sql;
+			PreparedStatement batchStatement = super.getBatchStatement( sql, callable );
+			createdStatements.add( batchStatement );
+			return batchStatement;
+		}
+
+		@Override
+		public void addToBatch() {
+			// Implementations really should call abortBatch() before throwing an exception.
+			// Purposely skipping the call to abortBatch() to ensure that Hibernate works properly when
+			// a legacy implementation does not call abortBatch().
+			throw sqlExceptionHelper().convert(
+					new SQLException( "fake SQLException" ),
+					"could not perform addBatch",
+					currentStatementSql
+			);
+		}
+
+		@Override
+		protected void releaseStatements() {
+			super.releaseStatements();
+			calledReleaseStatements = true;
+			numberOfStatementsAfterReleasing += getStatements().size();
+		}
+	}
+
+	@Override
+	protected String getBatchBuilderClassName() {
+		return TestBatchBuilder.class.getName();
+	}
+
+	public static class TestBatchBuilder extends BatchBuilderImpl {
+		private int jdbcBatchSize;
+
+		@Override
+		public void setJdbcBatchSize(int jdbcBatchSize) {
+			this.jdbcBatchSize = jdbcBatchSize;
+		}
+
+		@Override
+		public Batch buildBatch(BatchKey key, JdbcCoordinator jdbcCoordinator) {
+			return buildBatchTest( key, jdbcCoordinator, jdbcBatchSize );
+		}
+
+		protected BatchingBatch buildBatchTest(BatchKey key, JdbcCoordinator jdbcCoordinator, int jdbcBatchSize) {
+			testBatch = new TestBatch( key, jdbcCoordinator, jdbcBatchSize );
+			return testBatch;
+		}
+	}
+}