Request scope sometimes not propagated to ManagedExecutor.runAsync()

[wicksim]

Describe the bug
When executing code in ManagedExecutor.runAsync(), request-scoped beans are not always propagated. It often works on 1st call, but not on subsequent calls. quarkus-smallrye-context-propagation is active.

Expected behavior
Reqest-scoped beans should always be available if context is propagated.

Actual behavior
Request-scoped beans are available only sometimes.

To Reproduce
async-request-scope-problem.zip

Steps to reproduce the behavior:

1. mvn compile quarkus:dev
2. Look at System.out
3. Value is sometimes 5 (most often on first call) and sometimes null

Configuration
None

Screenshots
(If applicable, add screenshots to help explain your problem.)

Environment (please complete the following information):

• Output of uname -a or ver: Microsoft Windows [Version 10.0.17763.1457]
• Output of java -version: OpenJDK Runtime Environment Corretto-11.0.6.10.1 (build 11.0.6+10-LTS)
• GraalVM version (if different from Java): JVM-Mode
• Quarkus version or git rev: 1.9.1.Final
• Build tool (ie. output of mvnw --version or gradlew --version): Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)

Additional context
(Add any other context about the problem here.)

[geoand]

cc @manovotn @FroMage

[manovotn]

@wicksim I've spent some fiar time debugging and I think the problem lies elsewhere.
The short version is - I believe this use case shouldn't work, the only question is whether we should leave it as is, or throw an exception.
Let me try to explain in a (way) longer manner...

Posting the scheduled method here for clarity:

    @Scheduled(every = "1S")
    public void execute() {
        bean.setTest(5);
        System.out.println("Outside (Thread: " + Thread.currentThread().getId() + ", Bean: " + bean.toString() + "): " + bean.getTest());
        executor.runAsync(() -> {
            System.out.println("Inside (Thread: " + Thread.currentThread().getId() + ", Bean: " + bean.toString() + "): " + bean.getTest());
        });
    }

You're using @RequestScope bean - those normally require context to be active but in this case it is a courtesy of the scheduler that it activates the context for each @Scheduled method and deactivates it afterwards.
The catch is that the context is torn down as soon as the method exits. Therefore, in your example you are scheduling an async task to be run "sometime later" and then you leave the scheduled method, effectively killing the context and any request scoped bean with it.

Basically, whether the Inside call returns 5 or null depends on how fast it executes. If it comes too late, the bean is gone along with the information about the number. If it executes faster than the speed of context being torn down, it will pass and show you number 5, which you set into the bean. In other words, your case cannot work because it relies on request context being active where it is not guaranteed to be and precisely because the CDI context is being propagated, you're seeing the current state.

If you add @PostConstruct and @PreDestroy method inside the request scoped bean along with some System.out prints, you will see what I mean.

There's another issue here - why are you getting null when there is no bean anymore (as opposed to getting an exception)?
Context propagation for request context makes sure that each new thread(T1) about to run a task has request context active and is looking at the same bean storage as original thread(T0) - same ConcurrentHashMap. And when a context is destroyed (on T0), beans within that shared storage are cleared, leaving the map blank. However, other threads(T1) that had this map propagated don't have any idea if the original context was destroyed and all they see is a blank map and their context is still marked as active until their task is done. This means that an invocation like the one you do in your example will spawn another bean, living in T1's storage because in T0 this storage was already forgotten (or replaced with new one). In other words, we don't carry over the notion of context destruction to threads that had the map propagated.

I am not even sure we should do that, the only difference in this particular case would be an exception instead of null - you'd see a ContextNotActiveException. We'd probably have to share not just a map, but a whole concept of ContextState in between all threads and then be very defensive in always checking if the context is still active before retrieving/creating beans. But this just brings different issues such as locking of ContextState, any thread being able to shut down whole context, context becoming unavailable mid-way through some invocation because other thread killed it and so on...
WDYT @mkouba?

[wicksim]

I'll need a little bit more time to really understand your answer. 😃 But for now: My "real" case is not a scheduler, but an invocation of a REST-Endpoint. But I think this will not make any difference? I forgot to mention in the bug report that the scheduler was just an easy way for the reproducer.

[wicksim]

And by the way: Is there another preferred way to execute an async task in which the context propagation works corrrectly?

[manovotn]

But I think this will not make any difference?

Well, I can't be certain without the actual code for that use case, but I'd say it is indeed similar. As in, launching an async to-be-done-later task might mean that it would happen after the request is already over.

Is there another preferred way to execute an async task in which the context propagation works corrrectly?

Hm, I don't think this is about context propagation. Maybe I am poorly explaining this, but the issue lies in how request scope/context works in CDI. There is, by design, no way to use an information stored inside request scoped bean after its context has been shut down. You'd need to store that information in a bean (or elsewhere) that outlives destruction of request context (such as application scoped bean). Alternatively, you'd have to perform that async operation in a manner where you know it ends before you leave the request.

Context propagation for CDI works so that while original thread has req. context active, so will any other thread trying to use it. But it is not intended as means of outliving the original thread's context. That would defeat the point of having a limited scope on a bean.

[wicksim]

Okay, so I do understand: Context propagation is about propagating the context (incl. request scoped beans, transactions, etc.) to other threads, but only as long as the original thread lives.

But what about reactive programming (Mutiny)? Isn't it the case there, that things are executed async? So no request scoped information, too? Sorry, I'm kind of a newbie to this reactive thing...

So if I need to execute anything async without waiting for it in the original thread, I need to clone the request scoped bean or map the content to local variables in the original thread and use this variables in the async executed code?

[manovotn]

We should probably try to summarize this limitation of CDI context propagation in the documentation (along with the fact that due to this, one shouldn't rely on @PreDestroy callbacks in their beans).

But what about reactive programming (Mutiny)? Isn't it the case there, that things are executed async? So no request scoped information, too? Sorry, I'm kind of a newbie to this reactive thing...

I think the trick is that the original thread never destroys request context in this case hence you can keep using it inside CP. But I am not sure as I don't use reactive myself. Maybe @FroMage or @cescoffier would know?

[FroMage]

If you do this in a REST endpoint and you your return type is an async type such as CompletionStage or Uni then the request context will not be torn down until after your async request completes. This is the sort of async task that this is made to work for, where you do your thing async, and the server doesn't need to keep your thread around for it to be completed.

But the client will not get a REST response back until after your request completes.

[wicksim]

OK, I think I understand. One last question: If I need to perform a really async task (in the meaning of: the user gets an immediate response), the only possibility is to copy the relevant values from the request scoped bean to a local variable in the original thread an use this variable in the async execution, right? Or is there any other recommended way of doing this?

Shall I close this bug or will there be any documentation change?

[manovotn]

the only possibility is to copy the relevant values from the request scoped bean to a local variable in the original thread an use this variable in the async execution, right? Or is there any other recommended way of doing this?

As far as CDI context propagation goes, you could only use something like @ApplicationScoped bean. Such beans are visible from all threads and outlive HTTP request hence you wouldn't see this kind of problems.
Other than that, context propagation won't really help you proliferate state of data to other threads. In order words, your solution is as good as any :-)

Shall I close this bug or will there be any documentation change?

I'll try to add at least some documentation regarding the pre destroy so keep it open please.