jdaugherty commented on PR #16223: URL: https://github.com/apache/grails-core/pull/16223#issuecomment-5416296552
Update: I've taken the decorator fix from #16226 into this PR rather than leaving the plain revert — it addresses the cause (a callback attached during dispatch reuses the active `AsyncGrailsWebRequest` instead of asking for a second cycle), which is what the revert alone couldn't do. 2be110a4ed, credited to @codeconsole, including both of the new `WebPromisesSpec` features and the revised lookup-strategy spec asserting the refusal is reported rather than swallowed. One adjustment on top, in bc41fa9274. The fix changed the guard on running a decorated task from `request.isAsyncStarted()` to `!asyncRequest.isAsyncComplete()` — for every task. Those two differ in a real window: `isAsyncStarted()` goes false the moment the cycle is dispatched or completed, while `isAsyncComplete()` only flips when the container's `onComplete` listener fires. The loosening is exactly what the reuse path needs — during delivery, `isAsyncStarted()` is already false although the request is still live — but applied to the path that starts its own cycle, it widened when tasks run: between `dispatch()`/`complete()` and `onComplete`, a task that would previously have refused now bound a request the container may be recycling. So the guard is now split by how the decorator got its cycle, recorded at construction: - **started here** → `request.isAsyncStarted()` required — byte-for-byte the pre-#16192 semantics for the only path that existed then; - **joined in flight** → `!isAsyncComplete()` only — the exemption the delivery window exists for. The completion check also moved ahead of `asyncContext.request`: a completed `AsyncContext` throws its own message from `getRequest()`, which would have preempted the intended one. `AsyncWebRequestPromiseDecoratorSpec` pins all four corners — started-and-running runs, started-and-dispatched refuses, joined-in-delivery runs, joined-then-completed refuses. The started-and-dispatched feature fails against the single loosened guard (verified by removing the split and re-running), so the restored semantics are load-bearing, not decorative. Both #16226 `WebPromisesSpec` features still pass, so the reuse behaviour is intact. One honest caveat: a joined cycle can still complete while the task body is executing. That race predates #16192 — the original code had the same check-then-run shape — and can't be closed from the decorator; the guard's job here is only not to widen it. With this, the only remaining difference from #16226 is `WebUtils.clearGrailsWebRequest` — `try`/`finally` with the failure propagating, versus WARN-and-continue — per the discussion above. -- 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]
