This is an initial specification text. If we agree on that, I volunteer to provide TCK tests.

This basically means moving Context Propagation integration out of 3.0, and I remember you wanted Context Propagation in 3.0, so why the change of mind? I thought we all reached a consensus on the call today, and I tried to capture that consensus in this PR.

@Ladicek What I meant your texts on Context Propagation, I have raised this issue . They will discuss it on their next call. We then have a confirmed value on what is propaged.

MicroProfile Context Propagation intentionally doesn't define which contexts are propagated and which are cleared by default.
This is left to implementations.

This might not be correct if some conclusion can be reached for the above issue.

This basically means moving Context Propagation integration out of 3.0, and I remember you wanted Context Propagation in 3.0, so why the change of mind? I thought we all reached a consensus on the call today, and I tried to capture that consensus in this PR.

Not really, they will discuss the minimum set in their next meeting.

I added that as a note to help users. I'm fine with deleting the note.

Not really, they will discuss the minimum set in their next meeting.

From Fault Tolerance specification perspective, I couldn't care less. What matters is the released specification. If you think Context Propagation folks can, in a matter of few weeks, reach an agreement on something they couldn't agree on for months, then sure, but I have my doubts ;-)

I think this PR makes sense, otherwise you basically won't get MP CP into FT in time.
I share the doubts that MP CP will reach that conclusion/agreement in time (if at all).

I tried to expand the note to make the relationship with ConProp more explicit.

Added a TCK test. SmallRye Fault Tolerance passes this TCK as is and the test is skipped, and can be easily adjusted to pass this test by adding a few dependencies to the TCK runner module (because we already implement Context Propagation integration in the way it's specified here).

I ran into a problem implementing this.

It seems like in practise there's a limitation to the way that Context Propagation implementations propagate CDI scopes. Rather than having the same context active on both threads, they end up making a new context which references the same beans. This is often the same, but breaks if a bean gets initialized on the async thread. That bean instance is then only on the async thread and not on the original thread.

Details on the limitation here: eclipse/microprofile-context-propagation#167

We have existing tests which do exactly this in AsynchronousTest

Although the tests don't actually check that the context is propagated correctly, openliberty notices that this situation occurs and throws an exception. If we're adding integration with Context Propagation, we should update these tests so that they don't use this pattern which MP Context Propagation doesn't cope with correctly. We would need to make a call to the request scoped bean before we make the @Asynchronous call to ensure that it's initialized.

As a side note, I'm pretty disappointed with this limitation, I think it makes Context Propagation far less useful since the initialization time of CDI beans isn't something the user is supposed to have to worry about when using CDI.

Interesting. My understanding is that this is underspecified in MP Context Propagation -- I think in Quarkus, the request context is shared between threads, and it's a compatible implementation as well. So this is an implementation decision. I agree that's yet another rather disappointing fact of life :-)

You are both correct, here is some related info from the top of my head; bits might be missing :)

@Azquelt In Weld, what we do (simplified) is that we create new context on the thread and feed it same instances. This works for most cases but breaks whenever you need what I will call "backward propagation" (there is likely better term for that) - a situation where thread A creates another bean in given context and thread B also wants to access that. In this scenario they both end up creating their own instances.

There are several reasons why it is like this.

• Firstly, the initial set of use cases in CP didn't require this, we even specifically discussed it and I pointed it out several times.
• Secondly, the implementation:
  • For req/session/conversation Weld stores the beans in the request (or session) and when you enter new thread, you no longer have access to that request. Hence we spawn a new context, one that doesn't require backing http request.
  • Lifecycle of those scopes is also bound to that of a request/session and the new thread lacks knowledge of that; it also brings up the question of which thread is eligible to destroy the context, or what happens if one thread destroys it and another keeps working on it.
  • Even if you did somehow implement it, it would require heavy synchronization in the bean stores which is an overhead you surely don't want to have outside of CP scenarios.
  • It requires users to write their beans in a thread-safe manner which I highly doubt they do for anything other than app scoped beans, but that can already be an issue; this just underlines it.

@Ladicek Finally, in Quarkus the imlementation of req. context is far simpler and we control the usage - e.g. knowing how we allow users to propagate contexts and that you in fact won't be working on the same context in multiple threads (unless you enforce it) allows us to share the whole context instance, bean store included. Also note that in Quarkus, we do not store those beans in http request.

With all that being said, I am not against doing improvements on Weld side so long as we can keep the non-CP functionality and performance unhindered. So if anyone is willing to dive into this, contributions are welcome and I'll try to assist them as much as I can. Thought it is not going to be an easy task.

CC @mkouba in case you want to add some details I forgot.

From you observed @Azquelt you will have a brandnew requested scoped instance on the new thread. It will be the same content but a different instance. Though it is not ideal, it is ok. How about SessionScoped and ConversationScoped? Does Weld inject a new bean as well @manovotn ?

All propagated scopes work the same way in Weld.

@manovotn Thanks for the writeup, that's really helpful and filled in some gaps in my understanding.

@Ladicek @Emily-Jiang If this limitation is permitted by context propagation, should we adapt our existing TCK test to allow it?

Also, if Context Propagation has its own idea of what it does with the CDI context, do we still want to state the the request context is active in asynchronous methods or do we delegate that to Context Propagation as well?

Good point re "request context is active in @Async methods" -- I totally need to add a sentence to the new spec about that. (Basically, if ConProp is present, then this doesn't apply and ConProp takes over.)

When it comes to the existing @Async tests that use the request context... with the exception of tests that verify that request context is active in @Async methods (or generally with the exception of tests that specifically verify something related to request context), would it be best to just use @Dependent?

Added one tiny commit with a note that in presence of Context Propagation, the request context is still active (and also propagated).

Good point re "request context is active in @async methods" -- I totally need to add a sentence to the new spec about that. (Basically, if ConProp is present, then this doesn't apply and ConProp takes over.)

When it comes to the existing @async tests that use the request context... with the exception of tests that verify that request context is active in @async methods (or generally with the exception of tests that specifically verify something related to request context), would it be best to just use @dependent?

This has a serious impact to the end users. This means the method with @Asynchronous annotated are no longer support @RequestScoped, @ConversationScoped, etc, which is unacceptable limitation. With this hugh limitation, I can't see any value added by integrating with MP Context Propagation. With this said I am -1 to this integration until Weld can provide a working CDI context propagation without asking for the bean being initialised prior to the asynchronous calls.

I'm fine with deferring this to a later release because we find the Context Propagation specification unsatisfactory.

At the same time, I would like to point out one thing:

This has a serious impact to the end users. This means the method with @asynchronous annotated are no longer support @RequestScoped, @ConversationScoped, etc, which is unacceptable limitation.

This is simply not true.