jdaugherty commented on code in PR #16415:
URL: https://github.com/apache/grails-core/pull/16415#discussion_r4124230808
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/query/MongoQuery.java:
##########
@@ -464,8 +464,12 @@ public MongoQuery(AbstractMongoSession session,
PersistentEntity entity) {
@Override
protected void flushBeforeQuery() {
- // with Mongo we only flush the session if a transaction is not active
to allow for session-managed transactions
- if (!TransactionSynchronizationManager.isSynchronizationActive()) {
+ // Within a transaction the session is not flushed ahead of a query,
so that a rollback can still
+ // discard what is queued: without a server-side transaction, a
flushed write cannot be taken
+ // back. Inside one it is aborted with the transaction, so the query
sees the transaction's own
+ // writes, as on Hibernate.
+ if (!TransactionSynchronizationManager.isSynchronizationActive() ||
+ (mongoSession != null && mongoSession.hasActiveTransaction()))
{
super.flushBeforeQuery();
Review Comment:
**[P1] Prevent callback queries from re-entering an active flush**
With `grails.mongodb.transactional=true`, this new branch makes an ordinary
GORM query in `beforeInsert` recursively flush the same pending insert until
`StackOverflowError`. For example:
```groovy
@Entity
class Example {
String name
boolean beforeInsert() {
Example.count()
true
}
}
Example.withTransaction {
new Example(name: 'example').save(failOnError: true)
}
```
The commit flush calls `MongoCodecSession.flush()`, which invokes
`insert.run()` while the insert is still pending. The callback's `count()` now
reaches `super.flushBeforeQuery()`, which starts another flush and invokes the
same callback again. The Mongo codec flush path bypasses
`AbstractSession.flush()` and its `flushActive` guard. This is an implicit
flush caused by a read, not an explicit `save(flush: true)` inside the callback.
I reproduced this through the public GORM APIs against a real embedded
replica set. The same test passes with transactions both enabled and disabled
at the previously reviewed `5f98897e8a`. At `ff3dc8cef6`, the disabled case
still passes, but the enabled case fails with `StackOverflowError`; the
repeating stack is `MongoCodecSession.flush -> beforeInsert -> count ->
MongoQuery.flushBeforeQuery -> MongoCodecSession.flush`.
Please protect the Mongo flush path against reentry and add callback-query
regression coverage before enabling auto-flush here. Alternatively, defer the
optional query-flush enhancement from this PR. The existing new query tests do
not exercise persistence callbacks.
--
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]
