jdaugherty commented on PR #16344:
URL: https://github.com/apache/grails-core/pull/16344#issuecomment-5735093643
@matrei I have more changes since ddd5f0e38a Reviewing your feedback
revealed more problems. This could probably be a separate PR since it's only
tangentially related, but since Walter fixed the other issue identified, I
figured I'd include this too.
Three related changes in cde2e1007d. Two are behaviour changes, both
documented in the 8.0 upgrade notes.
1. mutex reloads under the lock
entity.mutex { } took an exclusive lock on the instance's already-loaded
state and ran the closure:
def <T> T mutex(D instance, Closure<T> callable) {
execute({ Session session ->
session.lock(instance) // -> hibernateTemplate.lock(o,
PESSIMISTIC_WRITE)
callable?.call()
} as SessionCallback)
}
That lock is version-checked, so a transaction committing to the row
between the load and the mutex call failed it with an optimistic locking
failure and the closure never ran — the same defect this PR removed from
refresh(lock: true) and lock(id, refresh: true). Neither Hibernate module
overrode mutex, so both were affected.
Where the datastore reports supportsLockedRefresh(), mutex now acquires
the lock without a version check and reloads the row under it, so it waits for
the competing writer and the closure runs on the committed state.
The lock stays exclusive and takes no lock-mode argument — a shared or
optimistic lock would not give the closure mutual exclusion, which is the point
of the method. Consequences for existing code, all in upgrade note 70:
unflushed changes to the instance are discarded (make changes inside the
closure), an active transaction is required, and the instance must be attached.
Datastores that cannot reload under a lock keep their previous behaviour.
2. Named-connection instance operations resolve their own connection
Reaching an instance operation through a named-connection static API
resolved the default connection's instance API. With only the named
connection's session open — which is how such code is written — it failed:
Book.secondary.withNewSession {
Book.secondary.withTransaction {
Book.secondary.save(book) //
org.hibernate.HibernateException: No Session found for current thread
}
}
GormStaticApi now passes the qualifier it was created with for every
instance operation (save, insert, merge, delete, attach, isAttached, discard,
ident, lock, mutex, instanceOf), as refresh already did. The instance form
book.secondary.save() was always correct and is unchanged.
3. Hibernate 5 records its connection qualifier
Change 2 alone fixed nothing on Hibernate 5, because its static API
discarded the qualifier it was constructed with.
HibernateGormApiFactory.createStaticApi and HibernateGormEnhancer.getStaticApi
both had it to hand and forwarded neither, and the constructor chain ends at
the deprecated GormStaticApi(Class, Datastore, List,
PlatformTransactionManager), which records ConnectionSource.DEFAULT.
Measured before the fix, for an entity mapped datasource 'ALL':
Book.secondary.qualifier = default # wrong
Book.secondary.qualifier = default # wrong
Book.secondary.datastore SF = 1992744664 # secondary
findInstanceApi(Book, 'secondary') SF = 1992744664 # the right api
existed all along
findInstanceApi(Book, null) SF = 1547127622
So a correctly-built secondary instance API was present; the static API
simply never asked for it. HibernateGormStaticApi now keeps its qualifier and
overrides getQualifier(), matching what GORM for Hibernate 7 already did.
Anything reading the qualifier of a Hibernate 5 named-connection static API now
sees the real connection name where it previously saw default.
Documentation
- Upgrade note 69 rewritten to cover every affected operation plus the
Hibernate 5 half.
- New upgrade note 70 for mutex, with a before/after snippet.
- mutex gained a section in both locking guides — it had no user
documentation at all.
--
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]
