matrei commented on PR #16344:
URL: https://github.com/apache/grails-core/pull/16344#issuecomment-5711138047

   ## Hibernate 7.4 defect behind the `refresh(entity, PESSIMISTIC_WRITE)` 
NullPointerException in #16344
   
   This documents the upstream Hibernate ORM behaviour that 
`HibernateGormInstanceApi.refresh(D, Map)` works around on Hibernate 7, so the 
reasoning survives the PR. Everything below was verified against Hibernate ORM 
7.4.7.Final with H2 2.4.240, using native `Session` calls only (no GORM in the 
failing path), and cross-checked against the 7.4.8.Final sources and the 
current `main` branch.
   
   ### Summary
   
   - `session.refresh(parent, LockModeType.PESSIMISTIC_WRITE)` throws a 
`NullPointerException` inside Hibernate when the parent has a refresh-cascaded 
association that is join-fetched, the dialect cannot lock outer-joined rows 
(H2, PostgreSQL), and the associated entity is already initialized in the 
persistence context.
   - `Locking.FollowOn.IGNORE` avoids the exception but silently drops the `FOR 
UPDATE` clause, so nothing is locked while `getCurrentLockMode` still reports 
`PESSIMISTIC_WRITE`. `Locking.FollowOn.DISALLOW` fails loudly with 
`IllegalQueryOperationException: Locking with OUTER joins is not supported`.
   - There is no exact upstream ticket. The same unguarded dereference is 
reported at a different call site as **HHH-20761**. The join-fetching 
cascade-refresh path that reaches it was introduced by **HHH-18774** in 
7.4.0.CR1. The follow-on locking semantics are discussed in **HHH-19211**.
   - Neither 7.4.8.Final nor `main` changes the failing line, so upgrading does 
not help.
   
   ### Upstream tickets
   
   | Ticket | Title | Status (2026-09-17) | Relevance |
   |---|---|---|---|
   | [HHH-20761](https://hibernate.atlassian.net/browse/HHH-20761) | NPE on 
entityInitializer | New, Major, affects 7.4.5 | Same root cause: 
`EntityHolder#getEntityInitializer()` is `@Nullable` but dereferenced. Their 
stack goes through `SubselectFetch$StandardRegistrationHandler.addKey` from a 
collection loader; ours goes through 
`JdbcValuesSourceProcessingStateStandardImpl.registerLoadingEntityHolder` from 
a locked refresh. On `main` the `SubselectFetch` site is wrapped in 
`castNonNull`, so upstream currently treats the null as impossible. |
   | [HHH-18774](https://hibernate.atlassian.net/browse/HHH-18774) | two bad 
bugs in cascade refresh | Closed, fix 7.4.0.CR1 | Rewrote cascade REFRESH so 
the refresh issues one query with a join and refreshes children already in the 
persistence context. That is the path that now registers an already-initialized 
child during a locked refresh. Versions before 7.4.0.CR1 do not take it. |
   | [HHH-19211](https://hibernate.atlassian.net/browse/HHH-19211) | 
Follow-on-locking silently breaks pessimistic lock semantics | Planning, Major 
| Design discussion of follow-on locking (the fallback that installs the 
`LoadedValuesCollector`). Confirms that the fallback is chosen implicitly for 
outer joins on dialects that cannot lock them, and that the Hibernate team is 
aware it is surprising. |
   
   ### Mechanism (Hibernate ORM 7.4.7.Final sources)
   
   1. `DefaultRefreshEventListener` runs `persister.load(id, object, 
lockOptions, source)` with the REFRESH cascading fetch profile enabled. The 
load plan join-fetches every refresh-cascaded association, lazy or not:
      ```sql
      select p.id, c.id, c.title, c.version, p.title, p.version
      from npe_eager_parent p left join npe_child c on c.id = p.child_id
      where p.id = ?
      ```
   2. `AbstractSqlAstTranslator.determineLockingStrategy` (line 1960 ff.) sees 
an outer join and `dialect.supportsOuterJoinForUpdate() == false` 
(`H2LockingSupport.getOuterJoinLockingType()` returns `IGNORED`; PostgreSQL 
likewise). With the default `FollowOn.ALLOW` it returns 
`LockStrategy.FOLLOW_ON`; with `DISALLOW` it throws; with `IGNORE` it returns 
`LockStrategy.NONE`, which is why no `FOR UPDATE` is emitted in that mode.
   3. `FOLLOW_ON` installs a `LoadedValuesCollector` 
(`FollowOnLockingAction.java:273`) so entities read by the query can be locked 
by a second statement afterwards.
   4. While reading the row, `EntityInitializerImpl` resolves the child. 
`StatefulPersistenceContext.claimEntityHolderIfPossible` (line 395 ff.) refuses 
to attach an initializer to a holder whose entity is already initialized (`if 
(oldHolder.isInitialized()) return oldHolder;`), so 
`holder.getEntityInitializer()` stays `null`. The accessor is declared 
`@Nullable` on `EntityHolder`.
   5. Because `isRefreshing(data)` is true (`isRefreshingCascadeAssociation`, 
line 693), the initializer takes the reload branch instead of the "already 
initialized, skip" branch (`EntityInitializerImpl.java:1338-1350`) and calls 
`registerLoadingEntityHolder(data.entityHolder)` (line 1695).
   6. `JdbcValuesSourceProcessingStateStandardImpl.registerLoadingEntityHolder` 
(line 91-107) dereferences `holder.getEntityInitializer().getNavigablePath()` 
without a null check, but only when `loadedValuesCollector != null`. That is 
why both follow-on locking and an already-initialized child are required.
   
   ### Trigger matrix
   
   Native `Session` calls inside a transaction, H2, parent mapped with `child 
cascade: 'all'`:
   
   | Case | Outcome |
   |---|---|
   | Eager child, parent and child initialized in the session, `refresh(parent, 
PESSIMISTIC_WRITE)` | **NullPointerException** (stack below) |
   | Lazy child, uninitialized proxy in the session | OK. Follow-on lock 
issued: `select tbl.id, ... from npe_lazy_parent tbl where tbl.id in (?) for 
update` |
   | Lazy child, proxy evicted from the session | OK |
   | Eager child, `LockOptions.setFollowOnStrategy(Locking.FollowOn.DISALLOW)` 
| `IllegalQueryOperationException: Locking with OUTER joins is not supported` |
   | Eager child, `LockOptions.setFollowOnStrategy(Locking.FollowOn.IGNORE)` | 
Succeeds, `getCurrentLockMode == PESSIMISTIC_WRITE`, but the SQL is the left 
join with **no `for update`**; a raw JDBC `select ... for update` on another 
connection is granted immediately |
   | Eager child, parent not in the session, `find(Parent, id, 
PESSIMISTIC_WRITE)` | OK. `select ... from npe_eager_parent where id=? for 
update`, then a separate unlocked select for the child |
   | Eager child, plain `refresh(parent)` | OK |
   
   ### Stack trace (7.4.7.Final)
   
   ```
   java.lang.NullPointerException: Cannot invoke 
"org.hibernate.sql.results.graph.entity.EntityInitializer.getNavigablePath()" 
because the return value of 
"org.hibernate.engine.spi.EntityHolder.getEntityInitializer()" is null
         at 
org.hibernate.sql.results.jdbc.internal.JdbcValuesSourceProcessingStateStandardImpl.registerLoadingEntityHolder(JdbcValuesSourceProcessingStateStandardImpl.java:98)
         at 
org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.initializeEntityInstance(EntityInitializerImpl.java:1695)
         at 
org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.initializeInstance(EntityInitializerImpl.java:1597)
         at 
org.hibernate.sql.results.graph.entity.internal.EntityInitializerImpl.initializeInstance(EntityInitializerImpl.java:88)
         at 
org.hibernate.sql.results.internal.StandardRowReader.coordinateInitializers(StandardRowReader.java:239)
         at 
org.hibernate.sql.results.internal.StandardRowReader.readRow(StandardRowReader.java:137)
         at 
org.hibernate.sql.results.spi.ListResultsConsumer.readUnique(ListResultsConsumer.java:279)
         at 
org.hibernate.sql.results.spi.ListResultsConsumer.readRows(ListResultsConsumer.java:227)
         at 
org.hibernate.sql.results.spi.ListResultsConsumer.consume(ListResultsConsumer.java:164)
         at 
org.hibernate.sql.results.spi.ListResultsConsumer.consume(ListResultsConsumer.java:31)
         at 
org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.doExecuteQuery(JdbcSelectExecutorStandardImpl.java:213)
         at 
org.hibernate.sql.exec.internal.JdbcSelectExecutorStandardImpl.executeQuery(JdbcSelectExecutorStandardImpl.java:100)
         at 
org.hibernate.sql.exec.spi.JdbcSelectExecutor.executeQuery(JdbcSelectExecutor.java:63)
         at 
org.hibernate.sql.exec.spi.JdbcSelectExecutor.list(JdbcSelectExecutor.java:137)
   ```
   
   ### Minimal reproducer for an upstream ticket (plain JPA, no GORM)
   
   ```java
   @Entity
   public class Parent {
       @Id @GeneratedValue Long id;
       @Version Long version;
       String title;
       @OneToOne(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
       Child child;
   }
   
   @Entity
   public class Child {
       @Id @GeneratedValue Long id;
       @Version Long version;
       String title;
   }
   
   // H2 or PostgreSQL, inside a transaction
   Parent parent = em.find(Parent.class, id);        // parent and child now 
initialized in the persistence context
   em.refresh(parent, LockModeType.PESSIMISTIC_WRITE); // NullPointerException
   ```
   
   Expected: the parent row is reloaded under a `FOR UPDATE` lock (follow-on or 
otherwise). Actual: `NullPointerException` from 
`JdbcValuesSourceProcessingStateStandardImpl.registerLoadingEntityHolder`. The 
same call succeeds when the child is an uninitialized proxy or has been 
evicted, and `em.find(Parent.class, id, LockModeType.PESSIMISTIC_WRITE)` with 
the parent not yet loaded works. Suggested title: "NPE in 
registerLoadingEntityHolder when refreshing with a pessimistic lock and a 
refresh-cascaded association is already initialized (follow-on locking)". 
Affected versions: 7.4.0.CR1 through 7.4.8.Final, `main` unchanged. Link 
HHH-20761 as the sibling report and HHH-18774 as the change that introduced the 
join-fetching cascade refresh.
   
   ### What #16344 does about it
   
   `HibernateGormInstanceApi.refresh(D, Map)` on Hibernate 7 no longer calls 
`session.refresh(instance, lockMode)` for pessimistic modes. It locks the row 
first with a scalar query (`select 1 from <entity> e where e = :instance` with 
the requested lock mode and `QueryFlushMode.NO_FLUSH`), then runs a plain 
`session.refresh(instance)`, then `session.lock(instance, lockMode)` to record 
the mode on the entity entry. The plain refresh carries no lock options, so 
`determineLockingStrategy` never selects follow-on locking and the code path 
above is not reached. A raw JDBC contender test in `Hibernate7RefreshLockSpec` 
observes the row lock directly, because `getCurrentLockMode` cannot distinguish 
a recorded lock from a held one.
   
   Hibernate 5.6 is unaffected: its locked refresh selects the parent `for 
update` and refreshes cascaded children with separate unlocked selects.
   
   ### Sources
   
   - [HHH-20761](https://hibernate.atlassian.net/browse/HHH-20761), 
[HHH-18774](https://hibernate.atlassian.net/browse/HHH-18774), 
[HHH-19211](https://hibernate.atlassian.net/browse/HHH-19211)
   - `hibernate-core-7.4.7.Final-sources.jar` and 
`hibernate-core-7.4.8.Final-sources.jar` from Maven Central; files 
`JdbcValuesSourceProcessingStateStandardImpl`, `EntityInitializerImpl`, 
`StatefulPersistenceContext`, `AbstractSqlAstTranslator`, 
`DefaultRefreshEventListener`, `H2LockingSupport`, `FollowOnLockingAction`
   - [`JdbcValuesSourceProcessingStateStandardImpl` on 
`main`](https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/JdbcValuesSourceProcessingStateStandardImpl.java),
 [`SubselectFetch` on 
`main`](https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/main/java/org/hibernate/engine/spi/SubselectFetch.java)
   - Hibernate release list from [Maven Central 
metadata](https://repo1.maven.org/maven2/org/hibernate/orm/hibernate-core/maven-metadata.xml):
 latest 7.4.x is 7.4.8.Final, plus 8.0.0.Beta1
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to