jdaugherty commented on code in PR #16797:
URL: https://github.com/apache/grails-core/pull/16797#discussion_r4419094817
##########
grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/GrailsWebRequestTaskDecorator.groovy:
##########
@@ -51,7 +55,9 @@ class GrailsWebRequestTaskDecorator implements TaskDecorator {
captured.currentRequest,
captured.currentResponse,
captured.attributes)
- WebUtils.storeGrailsWebRequest(taskRequest)
+ // Bound to the task's thread only: the request is still shared
with the request thread, which
+ // would find the task's web request on it from then on
+ RequestContextHolder.setRequestAttributes(taskRequest)
Review Comment:
Non-blocking. Worth adding the second reason to this comment: a task that
outlives the response, such as a fire-and-forget `task { }`, would otherwise
write its web request to a request object the container may already have
recycled for another request. Binding to the thread only closes that too.
##########
grails-web-common/src/main/groovy/org/grails/web/servlet/mvc/GrailsWebRequest.java:
##########
@@ -495,12 +497,36 @@ public PropertyEditorRegistry getPropertyEditorRegistry()
{
/**
* Looks up the GrailsWebRequest from the current request.
+ * <p>
+ * The web request bound to the current thread wins when it was built with
this request or another
+ * wrapper of the same request, so a task running alongside the request it
was started during, which
+ * is bound to a web request of its own, and the request thread each find
their own, however the
+ * request is wrapped. For any other request, the web request stored on it
is returned, falling back
+ * to the one bound to the thread.
+ *
* @param request The current request
* @return The GrailsWebRequest
*/
public static @Nullable GrailsWebRequest lookup(HttpServletRequest
request) {
+ GrailsWebRequest bound = lookup();
+ if (bound != null && sameRequest(bound.getRequest(), request)) {
Review Comment:
Record of what was checked here, no change asked. Every production caller of
this overload was traced. The include and forward paths
(`GrailsWebRequestFilter`, `UrlMappingUtils.includeForUrl` and
`forwardRequestForUrlMappingInfo`) set the thread binding and the attribute
together through `storeGrailsWebRequest`, so this preference hands back the
same object they stored. `GrailsDispatcherServlet.buildRequestAttributes` keeps
an active bound `GrailsWebRequest`, so on the request thread the bound one is
the filter's; under a plain `DispatcherServlet` (MockMvc) the bound attributes
are not a `GrailsWebRequest`, `lookup()` is null and the attribute path is
taken as before. The one path whose answer changes is the ASYNC re-dispatch,
where the attribute could hold a task's web request while the thread holds the
one the dispatcher built: the dispatcher's now wins, and it is the one handling
that dispatch.
--
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]
