codeconsole opened a new pull request, #16212:
URL: https://github.com/apache/grails-core/pull/16212
## Problem
`GormService` is annotated `@ReadOnly` at class level, so the transactional
AST transform wraps `get`/`list`/`count` in a `GrailsTransactionTemplate`. With
the default `REQUIRED` propagation, a read taken outside an existing
transaction becomes the **outermost** transaction — and committing it flushes
the session:
```java
// DatastoreTransactionManager.doCommit
if (!status.isReadOnly()) {
if (session != null) { ... session.flush(); } // skipped when read-only
}
transaction.commit(); // ...but this flushes
anyway
```
```java
// MongoTransaction.commit()
session.flush();
commitWithRetry();
```
So a read can write. `@ReadOnly` suppresses the transaction manager's own
flush, but not the one inside `commit()`.
That becomes a hang when such a read runs *during* a flush. A referential
check in a validator is the ordinary case:
```groovy
static constraints = {
principalId validator: { String val -> Principal.get(val) != null ?:
'principal.missing' }
}
```
If `Principal.get(...)` reaches `GormService`, the commit of its read-only
transaction flushes the session, the flush re-validates the entity being saved,
the validator reads again, and the cycle repeats until the stack is exhausted.
It surfaces well away from the cause — as `IllegalStateException: Transaction
synchronization is not active` thrown out of the commit unwind, with the
`StackOverflowError` lost. `beforeInsert`/`beforeUpdate` hooks are exposed the
same way.
## Change
Drop the class-level `@ReadOnly` from `GormService`. The write methods keep
their own `@Transactional`.
The annotation is redundant where scaffolded reads are actually served:
`RestfulServiceController` already declares `@ReadOnly` at class level, so that
path keeps its read-only boundary either way. The service annotation only takes
effect when the service is called *outside* one — which is exactly where
opening and committing a transaction is unwanted, and where callers reasonably
expect `get(id)` to behave like the GORM static call it delegates to.
## Tests
`:grails-scaffolding:test` plus `check` on
`:grails-test-examples-scaffolding`,
`:grails-test-examples-scaffolding-fields`, and
`:grails-test-examples-hibernate7-scaffolding-fields` (integration tests
included) all pass.
## Note
The unconditional flush in `MongoTransaction.commit()` is arguably a
separate defect — `@ReadOnly` reads as a guarantee that nothing will be
written, and on Mongo it isn't one. Relatedly,
`DatastoreTransactionManager.doBegin` sets `FlushModeType.COMMIT` for read-only
transactions while its comment says "Just set to NEVER". Happy to open a
follow-up if the behaviour is meant to be what the comment describes.
--
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]
