jdaugherty commented on code in PR #16344:
URL: https://github.com/apache/grails-core/pull/16344#discussion_r4056095621
##########
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormInstanceApi.groovy:
##########
@@ -136,7 +137,14 @@ class GormInstanceApi<D> extends AbstractGormApi<D>
implements GormInstanceOpera
@Override
def <T> T mutex(D instance, Closure<T> callable) {
execute({ Session session ->
- session.lock(instance)
+ if (supportsLockedRefresh()) {
+ // Reload the row under the lock instead of version-checking
the state already loaded, so that a
+ // competing writer is waited for and the closure runs on the
committed state. Always an
+ // exclusive lock: a shared or optimistic one would not give
the closure mutual exclusion.
+ refresh(instance, [(RefreshLockArguments.LOCK): true])
Review Comment:
I don't think we should keep the old behavior with mutex - it doesn't
actually lock and exceptions in highly concurrent environments in hibernate 5/7
(that's the whole reason this PR is making the change). I'll update the
documentation to call out the behavior difference & pin with test coverage.
--
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]
