From 0441fef53a97ee28ab403cebca3f6979b0fac9f6 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 16 Jul 2025 11:08:17 +0200 Subject: [PATCH 1/2] [#2332] SqlException, Reactive doesn't use JDBC when locking previously loaded entity --- .../DefaultReactiveLoadEventListener.java | 23 +-- .../ast/internal/ReactiveLoaderHelper.java | 143 ++++++++++++++++++ .../internal/ReactiveCacheLoadHelper.java | 46 ++++++ 3 files changed, 201 insertions(+), 11 deletions(-) create mode 100644 hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/internal/ReactiveCacheLoadHelper.java diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/event/impl/DefaultReactiveLoadEventListener.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/event/impl/DefaultReactiveLoadEventListener.java index 9804a02f6..4f3d53e58 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/event/impl/DefaultReactiveLoadEventListener.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/event/impl/DefaultReactiveLoadEventListener.java @@ -27,7 +27,6 @@ import org.hibernate.event.spi.EventSource; import org.hibernate.event.spi.LoadEvent; import org.hibernate.event.spi.LoadEventListener; -import org.hibernate.loader.internal.CacheLoadHelper; import org.hibernate.metamodel.mapping.AttributeMapping; import org.hibernate.metamodel.mapping.AttributeMappingsList; import org.hibernate.metamodel.mapping.CompositeIdentifierMapping; @@ -48,7 +47,7 @@ import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptable; import static org.hibernate.engine.internal.ManagedTypeHelper.isPersistentAttributeInterceptable; import static org.hibernate.loader.internal.CacheLoadHelper.loadFromSecondLevelCache; -import static org.hibernate.loader.internal.CacheLoadHelper.loadFromSessionCache; +import static org.hibernate.reactive.loader.internal.ReactiveCacheLoadHelper.loadFromSessionCache; import static org.hibernate.pretty.MessageHelper.infoString; import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer; import static org.hibernate.reactive.session.impl.SessionUtil.checkEntityFound; @@ -653,15 +652,17 @@ private CompletionStage doLoad( return nullFuture(); } else { - final CacheLoadHelper.PersistenceContextEntry persistenceContextEntry = - loadFromSessionCache( keyToLoad, event.getLockOptions(), options, event.getSession() ); - final Object entity = persistenceContextEntry.entity(); - if ( entity != null ) { - return persistenceContextEntry.isManaged() ? initializeIfNecessary( entity ) : nullFuture(); - } - else { - return loadFromCacheOrDatasource( event, persister, keyToLoad ); - } + return loadFromSessionCache( keyToLoad, event.getLockOptions(), options, event.getSession() ).thenCompose( + persistenceContextEntry -> { + final Object entity = persistenceContextEntry.entity(); + if ( entity != null ) { + return persistenceContextEntry.isManaged() ? initializeIfNecessary( entity ) : nullFuture(); + } + else { + return loadFromCacheOrDatasource( event, persister, keyToLoad ); + } + } + ); } } diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/ast/internal/ReactiveLoaderHelper.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/ast/internal/ReactiveLoaderHelper.java index 44c90bfdf..e15c42956 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/ast/internal/ReactiveLoaderHelper.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/ast/internal/ReactiveLoaderHelper.java @@ -9,10 +9,23 @@ import java.util.List; import java.util.concurrent.CompletionStage; +import org.hibernate.Hibernate; +import org.hibernate.LockMode; import org.hibernate.LockOptions; +import org.hibernate.ObjectDeletedException; +import org.hibernate.cache.spi.access.EntityDataAccess; +import org.hibernate.cache.spi.access.SoftLock; +import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.SubselectFetch; +import org.hibernate.event.monitor.spi.DiagnosticEvent; +import org.hibernate.event.monitor.spi.EventMonitor; +import org.hibernate.event.spi.EventSource; +import org.hibernate.internal.OptimisticLockHelper; +import org.hibernate.loader.LoaderLogging; import org.hibernate.metamodel.mapping.JdbcMapping; +import org.hibernate.pretty.MessageHelper; +import org.hibernate.reactive.persister.entity.impl.ReactiveEntityPersister; import org.hibernate.reactive.sql.exec.internal.StandardReactiveSelectExecutor; import org.hibernate.reactive.sql.results.spi.ReactiveListResultsConsumer; import org.hibernate.sql.ast.tree.expression.JdbcParameter; @@ -25,6 +38,8 @@ import org.hibernate.sql.results.internal.RowTransformerStandardImpl; import static java.util.Objects.requireNonNull; +import static org.hibernate.reactive.util.impl.CompletionStages.supplyStage; +import static org.hibernate.reactive.util.impl.CompletionStages.voidFuture; /** * @see org.hibernate.loader.ast.internal.LoaderHelper @@ -92,4 +107,132 @@ public static CompletionStage> loadByArrayParameter( ReactiveListResultsConsumer.UniqueSemantic.FILTER ); } + + /** + * A Reactive implementation of {@link org.hibernate.loader.ast.internal.LoaderHelper#upgradeLock(Object, EntityEntry, LockOptions, SharedSessionContractImplementor)} + */ + public static CompletionStage upgradeLock( + Object object, + EntityEntry entry, + LockOptions lockOptions, + SharedSessionContractImplementor session) { + final LockMode requestedLockMode = lockOptions.getLockMode(); + if ( requestedLockMode.greaterThan( entry.getLockMode() ) ) { + // Request is for a more restrictive lock than the lock already held + final ReactiveEntityPersister persister = (ReactiveEntityPersister) entry.getPersister(); + + if ( entry.getStatus().isDeletedOrGone()) { + throw new ObjectDeletedException( + "attempted to lock a deleted instance", + entry.getId(), + persister.getEntityName() + ); + } + + if ( LoaderLogging.LOADER_LOGGER.isTraceEnabled() ) { + LoaderLogging.LOADER_LOGGER.tracef( + "Locking `%s( %s )` in `%s` lock-mode", + persister.getEntityName(), + entry.getId(), + requestedLockMode + ); + } + + final boolean cachingEnabled = persister.canWriteToCache(); + SoftLock lock = null; + Object ck = null; + try { + if ( cachingEnabled ) { + final EntityDataAccess cache = persister.getCacheAccessStrategy(); + ck = cache.generateCacheKey( entry.getId(), persister, session.getFactory(), session.getTenantIdentifier() ); + lock = cache.lockItem( session, ck, entry.getVersion() ); + } + + if ( persister.isVersioned() && entry.getVersion() == null ) { + // This should be an empty entry created for an uninitialized bytecode proxy + if ( !Hibernate.isPropertyInitialized( object, persister.getVersionMapping().getPartName() ) ) { + Hibernate.initialize( object ); + entry = session.getPersistenceContextInternal().getEntry( object ); + assert entry.getVersion() != null; + } + else { + throw new IllegalStateException( String.format( + "Trying to lock versioned entity %s but found null version", + MessageHelper.infoString( persister.getEntityName(), entry.getId() ) + ) ); + } + } + + if ( persister.isVersioned() && requestedLockMode == LockMode.PESSIMISTIC_FORCE_INCREMENT ) { + // todo : should we check the current isolation mode explicitly? + OptimisticLockHelper.forceVersionIncrement( object, entry, session ); + } + else if ( entry.isExistsInDatabase() ) { + final EventMonitor eventMonitor = session.getEventMonitor(); + final DiagnosticEvent entityLockEvent = eventMonitor.beginEntityLockEvent(); + return reactiveLock( object, entry, lockOptions, session, persister, eventMonitor, entityLockEvent, cachingEnabled, ck, lock ); + } + else { + // should only be possible for a stateful session + if ( session instanceof EventSource eventSource ) { + eventSource.forceFlush( entry ); + } + } + entry.setLockMode(requestedLockMode); + } + finally { + // the database now holds a lock + the object is flushed from the cache, + // so release the soft lock + if ( cachingEnabled ) { + persister.getCacheAccessStrategy().unlockItem( session, ck, lock ); + } + } + } + return voidFuture(); + } + + private static CompletionStage reactiveLock( + Object object, + EntityEntry entry, + LockOptions lockOptions, + SharedSessionContractImplementor session, + ReactiveEntityPersister persister, + EventMonitor eventMonitor, + DiagnosticEvent entityLockEvent, + boolean cachingEnabled, + Object ck, + SoftLock lock) { + return supplyStage( () -> supplyStage( () -> persister.reactiveLock( entry.getId(), entry.getVersion(), object, lockOptions, session ) ) + .whenComplete( (v, e) -> completeLockEvent( entry, lockOptions, session, persister, eventMonitor, entityLockEvent, cachingEnabled, ck, lock, e == null ) ) ) + .whenComplete( (v, e) -> { + if ( cachingEnabled ) { + persister.getCacheAccessStrategy().unlockItem( session, ck, lock ); + } + } ); + } + + private static void completeLockEvent( + EntityEntry entry, + LockOptions lockOptions, + SharedSessionContractImplementor session, + ReactiveEntityPersister persister, + EventMonitor eventMonitor, + DiagnosticEvent entityLockEvent, + boolean cachingEnabled, + Object ck, + SoftLock lock, + boolean succes) { + eventMonitor.completeEntityLockEvent( + entityLockEvent, + entry.getId(), + persister.getEntityName(), + lockOptions.getLockMode(), + succes, + session + ); + if ( cachingEnabled ) { + persister.getCacheAccessStrategy().unlockItem( session, ck, lock ); + } + } + } diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/internal/ReactiveCacheLoadHelper.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/internal/ReactiveCacheLoadHelper.java new file mode 100644 index 000000000..1e6f3eddd --- /dev/null +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/loader/internal/ReactiveCacheLoadHelper.java @@ -0,0 +1,46 @@ +/* Hibernate, Relational Persistence for Idiomatic Java + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright: Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.reactive.loader.internal; + +import org.hibernate.LockOptions; +import org.hibernate.engine.spi.EntityEntry; +import org.hibernate.engine.spi.EntityKey; +import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.event.spi.LoadEventListener; +import org.hibernate.loader.internal.CacheLoadHelper; +import org.hibernate.loader.internal.CacheLoadHelper.PersistenceContextEntry.EntityStatus; + +import java.util.concurrent.CompletionStage; + +import static org.hibernate.loader.internal.CacheLoadHelper.PersistenceContextEntry.EntityStatus.MANAGED; +import static org.hibernate.reactive.loader.ast.internal.ReactiveLoaderHelper.upgradeLock; +import static org.hibernate.reactive.util.impl.CompletionStages.completedFuture; + +/** + * A reactive implementation of {@link CacheLoadHelper} + */ +public class ReactiveCacheLoadHelper { + + public static CompletionStage loadFromSessionCache( + EntityKey keyToLoad, + LockOptions lockOptions, + LoadEventListener.LoadType options, + SharedSessionContractImplementor session) { + final Object old = session.getEntityUsingInterceptor( keyToLoad ); + EntityStatus entityStatus = MANAGED; + if ( old != null ) { + // this object was already loaded + final EntityEntry oldEntry = session.getPersistenceContext().getEntry( old ); + entityStatus = CacheLoadHelper.entityStatus( keyToLoad, options, session, oldEntry, old ); + if ( entityStatus == MANAGED ) { + return upgradeLock( old, oldEntry, lockOptions, session ) + .thenApply(v -> new CacheLoadHelper.PersistenceContextEntry( old, MANAGED ) ); + } + } + return completedFuture( new CacheLoadHelper.PersistenceContextEntry( old, entityStatus ) ); + } + +} From e998de75ea4926de8596f510b2a284daa67c8483 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 16 Jul 2025 11:10:47 +0200 Subject: [PATCH 2/2] [#2332] Add test for SqlException, Reactive doesn't use JDBC when locking previously loaded entity --- .../hibernate/reactive/LockOnLoadTest.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/LockOnLoadTest.java diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/LockOnLoadTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/LockOnLoadTest.java new file mode 100644 index 000000000..7f946d04f --- /dev/null +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/LockOnLoadTest.java @@ -0,0 +1,67 @@ +/* Hibernate, Relational Persistence for Idiomatic Java + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright: Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.reactive; + +import org.hibernate.LockMode; + +import org.junit.jupiter.api.Test; + +import io.vertx.junit5.Timeout; +import io.vertx.junit5.VertxTestContext; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import java.util.Collection; +import java.util.List; + +import static java.util.concurrent.TimeUnit.MINUTES; +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +@Timeout(value = 10, timeUnit = MINUTES) +public class LockOnLoadTest extends BaseReactiveTest{ + @Override + protected Collection> annotatedEntities() { + return List.of( Person.class ); + } + + @Test + public void testLockOnLoad(VertxTestContext context) { + Person person = new Person( 1L, "Davide" ); + + test( context, getMutinySessionFactory() + .withTransaction( session -> session.persist( person ) ) + .call( () -> getMutinySessionFactory().withSession( session -> session + .find( Person.class, person.getId() ) + // the issue occurred when trying to find the same entity but upgrading the lock mode + .chain( p -> session.find( Person.class, person.getId(), LockMode.PESSIMISTIC_WRITE ) ) + .invoke( p -> assertThat( p ).isNotNull() ) + ) ) + ); + } + + @Entity(name = "Person") + public static class Person { + @Id + private Long id; + + private String name; + + public Person() { + } + + public Person(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public String getName() { + return name; + } + } +}