Hello,

we started to (sometimes) get following exception when trying to read request headers in another thread using RequestContextHolder

java.lang.RuntimeException: java.lang.NullPointerException: Cannot invoke "org.eclipse.jetty.server.Request.getHeaders()" because the return value of "org.eclipse.jetty.ee10.servlet.ServletApiRequest.getRequest()" is null
    at a.b.c.feign.AuthorizationHeaderInterceptor.getAuthorizationHeader(AuthorizationHeaderInterceptor.java:30)
    at a.b.c.feign.AuthorizationHeaderInterceptor.apply(AuthorizationHeaderInterceptor.java:19)
       ...

I have described the issue in good detail here

in short, starting from jetty 12.0.1 it now recycles more internal objects after request-response has finished, but org.springframework.web.context.request.RequestAttributes holds a reference to org.eclipse.jetty.ee10.servlet.ServletApiRequest. NPE may happen when request attributes are accessed from another thread

do I need to implement a caching wrapper around the org.eclipse.jetty.ee10.servlet.ServletApiRequest so I can access the attributes/headers without interacting with org.eclipse.jetty.ee10.servlet.ServletApiRequest outside the request processing loop?

Comment From: bclozel

I'm not sure I understand. The Jetty issue you've linked to claims that request objects are recycled before the async request is completed; as far as I understand, this is a bug as async exchanges can be completed from other threads. You issue description claims that Jetty "now recycles more internal objects after request-response has finished" - in this case, I would argue that you should never access such instances once the async request is completed and you should expect such behavior.

Are you completing the async request from within this wrapped Runnable?

Comment From: eugene-sadovsky

you should never access such instances once the async request is completed and you should expect such behavior

I agree, however after looking how spring-cloud-openfeign propagates request attributes, I was under the impression this is supposed to work out of the box, and it does for us with jetty 12.0.0, not saying this is the way go.

Just to be clear, this is what I have

@RestController
public class SomeController {

    @RequestMapping(value = {"/a/b/c"})
    public SomeDTO handleABC(){

    // will run on another thread, will need access to RequestAttributes
    service.runAsync()

        return new SomeDTO();
    };
}

service.runAsync() is annotated with @Async and we use TaskDecorator to propagate RequestAttributes:

import org.springframework.core.task.TaskDecorator;
import org.springframework.web.context.request.RequestAttributes;
import org.springframework.web.context.request.RequestContextHolder;

public class AsyncRequestAttributesPropagator implements TaskDecorator {

    @Override
    public Runnable decorate(Runnable runnable) {
        RequestAttributes parentRequestAttributes = RequestContextHolder.getRequestAttributes();
        return () -> {
            try {
                RequestContextHolder.setRequestAttributes(parentRequestAttributes);
                runnable.run();
            } finally {
                RequestContextHolder.resetRequestAttributes();
            }
        };
    }

further down the line, runAsync() tries to access request headers (inside feign interceptor for another outbound request) and fails, because now jetty nulls _request = _servletContextRequest = null; upon recycle. Looks like prior to 12.0.1 this was working "by accident", because _servletContextRequest was still available. And what's worse, may have even been associated with another request at the time of access (speculation)

My question is then: 1. wouldn't this cause issues for FeignCircuitBreakerInvocationHandler in async mode linked above ? 2. what is the recommended way to go about it? In may case I can think of implementing a wrapper around the original request, something like this and setting new RequestAttributes with it inside my task decorator

p.s. I may have commented on a wrong jetty issue now that I re-read it

Comment From: bclozel

Thanks for the detailed feedback @eugene-sadovsky !

Indeed, if you'd like your endpoint to respond right away and defer some other work in a different thread, this does not qualify as async handling in the Servlet sense. I think I would try and completely untangle this service.runAsync() from the request by creating a custom context instance that holds the information you want. Typically, you would extract/copy HTTP headers from the request and put those in the context instance.

The approach you're showing would only work if the controller returns a DeferredResult and that service.runAsync() completes the response once it's done. I guess this is not the case here.

I'm closing this issue for now, as I don't think we should change anything in Spring Framework.

Comment From: eugene-sadovsky

thank you for the answer 👍🏼