HHH-20260 Avoid swallowing lock exception in Session#find on PostgreSQL/Sybase

hibernate-core/src/main/java/org/hibernate/dialect/SybaseASEDialect.java

@@ -118,6 +118,21 @@ public SybaseASEDialect(DialectResolutionInfo info) {
 		pageSize = pageSize( info );
 	}
 
+	@Override
+	public DatabaseVersion determineDatabaseVersion(DialectResolutionInfo info) {
+		if ( SybaseDriverKind.determineKind( info ) == SybaseDriverKind.JTDS
+			&& info.getDatabaseMinorVersion() != DatabaseVersion.NO_VERSION ) {
+			// The jTDS driver encodes the SP part into the minor version, so we have to unpack this
+			final int infoMinorVersion = info.getDatabaseMinorVersion();
+			final int minorVersion = infoMinorVersion / 10;
+			final int microVersion = infoMinorVersion % 10;
+			return new SimpleDatabaseVersion( info.getDatabaseMajorVersion(), minorVersion, microVersion );
+		}
+		else {
+			return super.determineDatabaseVersion( info );
+		}
+	}
+
 	@Override
 	protected String columnType(int sqlTypeCode) {
 		return switch ( sqlTypeCode ) {

hibernate-core/src/main/java/org/hibernate/dialect/lock/internal/TransactSQLLockingSupport.java

@@ -47,8 +47,8 @@ public class TransactSQLLockingSupport extends LockingSupportParameterized {
 	public static final LockingSupport SYBASE_ASE = new TransactSQLLockingSupport(
 			PessimisticLockStyle.TABLE_HINT,
 			LockTimeoutType.CONNECTION,
-			LockTimeoutType.NONE,
-			LockTimeoutType.NONE,
+			LockTimeoutType.CONNECTION,
+			LockTimeoutType.QUERY,
 			RowLockStrategy.TABLE,
 			OuterJoinLockingType.IDENTIFIED,
 			SybaseImpl.IMPL

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

@@ -40,6 +40,8 @@
 import org.hibernate.event.service.spi.EventListenerGroups;
 import org.hibernate.event.spi.*;
 import org.hibernate.event.spi.LoadEventListener.LoadType;
+import org.hibernate.exception.GenericJDBCException;
+import org.hibernate.exception.JDBCConnectionException;
 import org.hibernate.graph.GraphSemantic;
 import org.hibernate.graph.spi.RootGraphImplementor;
 import org.hibernate.internal.find.FindByKeyOperation;
@@ -2189,8 +2191,11 @@ private <T> T find(Class<T> entityClass, Object primaryKey, LockOptions lockOpti
 			throw getExceptionConverter().convert( new IllegalArgumentException( e.getMessage(), e ) );
 		}
 		catch ( JDBCException e ) {
-			if ( accessTransaction().isActive() && accessTransaction().getRollbackOnly() ) {
-				// Assume situation HHH-12472 running on WildFly
+			final Transaction transaction = accessTransaction();
+			if ( transaction.isActive() && transaction.getRollbackOnly()
+				&& (e instanceof GenericJDBCException || e instanceof JDBCConnectionException) ) {
+				// Assume situation HHH-12472 running on WildFly,
+				// but only if the exception is generic to avoid swallowing locking exceptions (HHH-20260)
 				// Just log the exception and return null
 				SESSION_LOGGER.jdbcExceptionThrownWithTransactionRolledBack( e );
 				return null;

hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcSelectExecutorStandardImpl.java

@@ -125,6 +125,14 @@ private <T, R> T doExecuteQuery(
 			StatementCreator statementCreator,
 			ResultsConsumer<T, R> resultsConsumer) {
 
+		final var session = executionContext.getSession();
+		final var factory = session.getFactory();
+		final var logicalConnection = session.getJdbcCoordinator().getLogicalConnection();
+
+		final var connection = logicalConnection.getPhysicalConnection();
+		final var statementAccess = new StatementAccessImpl( connection, logicalConnection, factory );
+		jdbcSelect.performPreActions( statementAccess, connection, executionContext );
+
 		final var deferredResultSetAccess = new DeferredResultSetAccess(
 				jdbcSelect,
 				jdbcParameterBindings,
@@ -144,9 +152,6 @@ private <T, R> T doExecuteQuery(
 			rowTransformer = getRowTransformer( executionContext, jdbcValues );
 		}
 
-		final var session = executionContext.getSession();
-		final var factory = session.getFactory();
-
 		final boolean stats;
 		long startTime = 0;
 		final var statistics = factory.getStatistics();
@@ -201,12 +206,6 @@ public boolean shouldReturnProxies() {
 
 		final var rowProcessingState = new RowProcessingStateStandardImpl( valuesProcessingState, executionContext, rowReader, jdbcValues );
 
-		final var logicalConnection = session.getJdbcCoordinator().getLogicalConnection();
-
-		final var connection = logicalConnection.getPhysicalConnection();
-		final var statementAccess = new StatementAccessImpl( connection, logicalConnection, factory );
-		jdbcSelect.performPreActions( statementAccess, connection, executionContext );
-
 		try {
 			final T result = resultsConsumer.consume(
 					jdbcValues,

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockExceptionTests.java

@@ -13,6 +13,7 @@
 import org.hibernate.community.dialect.InformixDialect;
 import org.hibernate.dialect.CockroachDialect;
 import org.hibernate.dialect.SQLServerDialect;
+import org.hibernate.dialect.SybaseASEDialect;
 import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider;
 import org.hibernate.exception.LockAcquisitionException;
 import org.hibernate.orm.test.jpa.model.AbstractJPATest;
@@ -22,6 +23,7 @@
 import org.hibernate.testing.orm.junit.JiraKey;
 import org.hibernate.testing.orm.junit.RequiresDialectFeature;
 import org.hibernate.testing.orm.junit.SkipForDialect;
+import org.hibernate.testing.orm.junit.VersionMatchMode;
 import org.hibernate.testing.transaction.TransactionUtil2;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
@@ -54,6 +56,8 @@ protected void tearDown() {
 	@Test
 	@JiraKey( value = "HHH-8786" )
 	@SkipForDialect(dialectClass = InformixDialect.class, reason = "no failure")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testLockTimeoutFind() {
 		final Item item = new Item( "find" );
 
@@ -96,6 +100,8 @@ public void testLockTimeoutFind() {
 	@Test
 	@SkipForDialect(dialectClass = CockroachDialect.class, reason = "Cockroach uses SERIALIZABLE by default and seems to fail reading a row that is exclusively locked by a different TX")
 	@SkipForDialect(dialectClass = InformixDialect.class, reason = "Cursor must be on simple SELECT for FOR UPDATE")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testLockTimeoutRefresh() {
 		final Item item = new Item( "refresh" );
 
@@ -139,6 +145,8 @@ public void testLockTimeoutRefresh() {
 	@Test
 	@SkipForDialect(dialectClass = CockroachDialect.class, reason = "Cockroach uses SERIALIZABLE by default and seems to fail reading a row that is exclusively locked by a different TX")
 	@SkipForDialect(dialectClass = InformixDialect.class, reason = "no failure")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testLockTimeoutLock() {
 		final Item item = new Item( "lock" );
 

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/LockTest.java

@@ -35,12 +35,15 @@
 import org.hibernate.dialect.PostgreSQLDialect;
 import org.hibernate.dialect.SQLServerDialect;
 
+import org.hibernate.dialect.SybaseASEDialect;
+import org.hibernate.dialect.lock.PessimisticEntityLockException;
 import org.hibernate.testing.orm.junit.DialectFeatureChecks;
 import org.hibernate.testing.orm.junit.EntityManagerFactoryBasedFunctionalTest;
 import org.hibernate.testing.orm.junit.RequiresDialect;
 import org.hibernate.testing.orm.junit.RequiresDialectFeature;
 import org.hibernate.testing.orm.junit.SkipForDialect;
 import org.hibernate.testing.orm.junit.JiraKey;
+import org.hibernate.testing.orm.junit.VersionMatchMode;
 import org.hibernate.testing.transaction.TransactionUtil;
 import org.hibernate.testing.util.ExceptionUtil;
 import org.junit.jupiter.api.AfterEach;
@@ -113,10 +116,12 @@ public void testFindWithTimeoutHint() {
 
 	@Test
 	@JiraKey( value = "HHH-7252" )
-	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class,
+	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNoWait.class,
 							comment = "Test verifies proper exception throwing when a lock timeout is specified.",
 							jiraKey = "HHH-7252" )
 	@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Altibase close socket after lock timeout occurred")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testFindWithPessimisticWriteLockTimeoutException() {
 		assertTimeout( Duration.ofSeconds(5), () -> {
 			Lock lock = new Lock();
@@ -146,7 +151,12 @@ public void testFindWithPessimisticWriteLockTimeoutException() {
 							lte.getCause();
 						}
 						catch (PessimisticLockException pe) {
-							fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							// Per spec, the Hibernate LockTimeoutException is wrapped into a PessimisticLockException
+							// if the transaction was marked for rollback
+							if ( !entityManager.getTransaction().getRollbackOnly()
+								|| !(pe.getCause() instanceof org.hibernate.exception.LockTimeoutException) ) {
+								fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							}
 						}
 						catch (PersistenceException pe) {
 							log.info(
@@ -165,10 +175,12 @@ public void testFindWithPessimisticWriteLockTimeoutException() {
 
 	@Test
 	@JiraKey( value = "HHH-13364" )
-	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class,
+	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNoWait.class,
 			comment = "Test verifies proper exception throwing when a lock timeout is specified for Query#getSingleResult.",
 			jiraKey = "HHH-13364" )
 	@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Altibase close socket after lock timeout occurred")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testQuerySingleResultPessimisticWriteLockTimeoutException() {
 		assertTimeout( Duration.ofSeconds(5), () -> {
 			Lock lock = new Lock();
@@ -198,7 +210,12 @@ public void testQuerySingleResultPessimisticWriteLockTimeoutException() {
 							lte.getCause();
 						}
 						catch (PessimisticLockException pe) {
-							fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							// Per spec, the Hibernate LockTimeoutException is wrapped into a PessimisticLockException
+							// if the transaction was marked for rollback
+							if ( !entityManager.getTransaction().getRollbackOnly()
+								|| !(pe.getCause() instanceof org.hibernate.exception.LockTimeoutException) ) {
+								fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							}
 						}
 						catch (PersistenceException pe) {
 							log.info(
@@ -216,10 +233,12 @@ public void testQuerySingleResultPessimisticWriteLockTimeoutException() {
 
 	@Test
 	@JiraKey( value = "HHH-13364" )
-	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class,
+	@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNoWait.class,
 			comment = "Test verifies proper exception throwing when a lock timeout is specified for Query#getResultList.",
 			jiraKey = "HHH-13364" )
 	@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Altibase close socket after lock timeout occurred")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testQueryResultListPessimisticWriteLockTimeoutException() {
 		assertTimeout( Duration.ofSeconds(5), () -> {
 			Lock lock = new Lock();
@@ -249,7 +268,12 @@ public void testQueryResultListPessimisticWriteLockTimeoutException() {
 							lte.getCause();
 						}
 						catch (PessimisticLockException pe) {
-							fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							// Per spec, the Hibernate LockTimeoutException is wrapped into a PessimisticLockException
+							// if the transaction was marked for rollback
+							if ( !entityManager.getTransaction().getRollbackOnly()
+								|| !(pe.getCause() instanceof org.hibernate.exception.LockTimeoutException) ) {
+								fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							}
 						}
 						catch (PersistenceException pe) {
 							log.info(
@@ -272,6 +296,8 @@ public void testQueryResultListPessimisticWriteLockTimeoutException() {
 			comment = "Test verifies proper exception throwing when a lock timeout is specified for NamedQuery#getResultList.",
 			jiraKey = "HHH-13364" )
 	@SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Altibase close socket after lock timeout occurred")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	public void testNamedQueryResultListPessimisticWriteLockTimeoutException() {
 		assertTimeout( Duration.ofSeconds(5), () -> {
 			Lock lock = new Lock();
@@ -298,7 +324,12 @@ public void testNamedQueryResultListPessimisticWriteLockTimeoutException() {
 							lte.getCause();
 						}
 						catch (PessimisticLockException pe) {
-							fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							// Per spec, the Hibernate LockTimeoutException is wrapped into a PessimisticLockException
+							// if the transaction was marked for rollback
+							if ( !entityManager.getTransaction().getRollbackOnly()
+								|| !(pe.getCause() instanceof org.hibernate.exception.LockTimeoutException) ) {
+								fail( "Find with immediate timeout should have thrown LockTimeoutException." );
+							}
 						}
 						catch (PersistenceException pe) {
 							log.info(
@@ -895,6 +926,17 @@ public void testContendedPessimisticWriteLockNoWait() throws Exception {
 								log.info( "testContendedPessimisticWriteLockNoWait: (BG) got expected timeout exception" );
 								timedOut.set( true );
 							}
+							catch (PessimisticLockException pe) {
+								// Per spec, the Hibernate LockTimeoutException is wrapped into a PessimisticLockException
+								// if the transaction was marked for rollback
+								if ( _entityManager.getTransaction().getRollbackOnly()
+									&& pe.getCause() instanceof PessimisticEntityLockException cause
+									&& cause.getCause() instanceof org.hibernate.exception.LockTimeoutException) {
+									// success
+									log.info( "testContendedPessimisticWriteLockNoWait: (BG) got expected timeout exception" );
+									timedOut.set( true );
+								}
+							}
 							catch ( Throwable e ) {
 								log.info( "Expected LockTimeoutException but got unexpected exception", e );
 							}

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/StatementIsClosedAfterALockExceptionTest.java

@@ -15,12 +15,14 @@
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.community.dialect.AltibaseDialect;
 import org.hibernate.community.dialect.InformixDialect;
+import org.hibernate.dialect.SybaseASEDialect;
 import org.hibernate.testing.orm.jdbc.PreparedStatementSpyConnectionProvider;
 import org.hibernate.testing.orm.junit.DialectFeatureChecks;
 import org.hibernate.testing.orm.junit.RequiresDialectFeature;
 import org.hibernate.testing.orm.junit.SkipForDialect;
 import org.hibernate.testing.orm.junit.EntityManagerFactoryBasedFunctionalTest;
 import org.hibernate.testing.orm.junit.JiraKey;
+import org.hibernate.testing.orm.junit.VersionMatchMode;
 import org.hibernate.testing.transaction.TransactionUtil;
 import org.hibernate.testing.util.ExceptionUtil;
 import org.junit.jupiter.api.AfterEach;
@@ -37,8 +39,10 @@
 /**
  * @author Andrea Boriero
  */
-@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsLockTimeouts.class)
+@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNoWait.class)
 @SkipForDialect(dialectClass = AltibaseDialect.class, reason = "Altibase does not close Statement after lock timeout")
+@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+		versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 public class StatementIsClosedAfterALockExceptionTest extends EntityManagerFactoryBasedFunctionalTest {
 
 	private static final PreparedStatementSpyConnectionProvider CONNECTION_PROVIDER = new PreparedStatementSpyConnectionProvider();

hibernate-core/src/test/java/org/hibernate/orm/test/locking/LockingBasedOnSelectClauseTests.java

@@ -13,6 +13,7 @@
 import jakarta.persistence.Table;
 import org.hibernate.community.dialect.InformixDialect;
 import org.hibernate.dialect.HSQLDialect;
+import org.hibernate.dialect.SybaseASEDialect;
 import org.hibernate.spi.NavigablePath;
 import org.hibernate.sql.ast.tree.Statement;
 import org.hibernate.sql.ast.tree.select.SelectStatement;
@@ -21,6 +22,7 @@
 import org.hibernate.testing.orm.junit.SessionFactory;
 import org.hibernate.testing.orm.junit.SessionFactoryScope;
 import org.hibernate.testing.orm.junit.SkipForDialect;
+import org.hibernate.testing.orm.junit.VersionMatchMode;
 import org.hibernate.testing.orm.transaction.TransactionUtil;
 import org.hibernate.testing.util.ast.HqlHelper;
 import org.hibernate.testing.util.ast.LoadingAstHelper;
@@ -61,6 +63,8 @@ void tearDown(SessionFactoryScope factoryScope) {
 	@Test
 	@SkipForDialect(dialectClass = HSQLDialect.class, reason = "See https://sourceforge.net/p/hsqldb/bugs/1734/")
 	@SkipForDialect(dialectClass = InformixDialect.class, reason = "Cursor must be on simple SELECT for FOR UPDATE")
+	@SkipForDialect(dialectClass = SybaseASEDialect.class, majorVersion = 16, minorVersion = 0, microVersion = 2,
+			versionMatchMode = VersionMatchMode.SAME_OR_OLDER, reason = "holdlock isn't the same as updating a row. Bug in our Sybase ASE version?")
 	void testBasicHqlUsage(SessionFactoryScope factoryScope) {
 		factoryScope.inTransaction( (session) -> {
 			session.createQuery( "select b.author from Book b" )