jdaugherty commented on PR #16344: URL: https://github.com/apache/grails-core/pull/16344#issuecomment-5704733489
Thanks, both findings were real and finding 1 was worse than I'd understood. Fixed in `4198a9cb84`. **Finding 1.** `FollowOn.IGNORE` was chosen to avoid the 7.4 NullPointerException, on the reading that it skipped the follow-on selects but kept the main statement's lock. Your trace through `determineLockingStrategy` is right: with an outer join on a dialect that cannot lock joined rows it returns `LockStrategy.NONE`, so the parent row was never locked either. I reproduced the NPE with plain JPA entities and a bare `Configuration`, no GORM in the picture, and it also fires under `Locking.Scope.ROOT_ONLY`. The refresh statement join-fetches every refresh-cascaded association, lazy ones included, so the trigger is any such association whose entity is already in the persistence context. I could not find an upstream ticket and intend to file one. The fix takes your suggestion: lock the row first with a scalar HQL query for the entity (no joins, so a plain `FOR UPDATE` on every dialect, and no version check, so a stale instance is fine), then a plain refresh under the held lock, then `lock()` so the entity entry records the mode. The lock query runs with `NO_FLUSH`; otherwise AUTO flush writes the pending edit before the refresh discards it. Children are reloaded unlocked, which is what Hibernate 5 does natively. Your JDBC contender test is in both specs on the cascade parent, with a 300 ms lock timeout on a separate `DriverManager` connection. **Finding 2.** The dirty-state walk now dispatches on the Hibernate `Type`: components are descended into, collections visited element by element, and only entities are reset. The embedded-with-cascading-association case is tested and the What's New entry describes the Hibernate 5 reset. **Finding 3.** Both static APIs return the proxy when one is registered for the key, tested with an initialized proxy on both versions. **Finding 4.** The core static path passes the connection qualifier. I also added data-driven coverage of every `LockModeType` for `refresh(lock:)`, `lock(id, type:)` and `lock(id, refresh: true, type:)` on both versions, asserting the recorded mode and the version after commit. That turned up two Hibernate 5 gaps that are now fixed: optimistic modes after a refresh never registered their commit-time check or increment (they now go through `lock()`), and Hibernate 5 reports `PESSIMISTIC_FORCE_INCREMENT` under its legacy `FORCE` alias, which the spec expects. `type: NONE` remains rejected; `refresh(lock: NONE)` is a plain refresh and is tested as such. The reference pages, What's New and both Hibernate guides now carry a lock mode table. Full suites for the five modules pass (983 / 36 / 612 / 907 / 3147, no failures), `codeStyle` is clean on the four modules, and the main guide and both Hibernate guides build. PR title and description still need the rename; I'll do that separately. -- 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]
