jakartaee / cdi

CDI specification
Apache License 2.0
215 stars 78 forks source link

Allow relaxing invoker lookup rules for asynchronous methods #768

Closed Ladicek closed 9 months ago

Ladicek commented 9 months ago

This is a proposal for the one and only unresolved question that's left after #749.

I propose that implementations may expose an API for handling asynchronous methods when it comes to destroying @Dependent instances created to service the invocation.

When they do so, the current text includes a recommendation for how they may behave. We might want to turn that into a requirement: if the implementation decides to support asynchronous methods, they must do that in the prescribed way. WDYT?

CC @manovotn @Azquelt

manovotn commented 9 months ago

This is a proposal for the one and only unresolved question that's left after #749.

I propose that implementations may expose an API for handling asynchronous methods when it comes to destroying @Dependent instances created to service the invocation.

When they do so, the current text includes a recommendation for how they may behave. We might want to turn that into a requirement: if the implementation decides to support asynchronous methods, they must do that in the prescribed way. WDYT?

CC @manovotn @Azquelt

I'd keep it at recommendation level for now - we cannot enforce it anyway (via TCKs) and it still clearly shows the intent behind this wording.

starksm64 commented 9 months ago

Just a recommendation as it cannot be tested at this point. Even in the future, detail would need to be specified for both Java SE and EE environment as the later should have some integration with concurrency in the absence of a common asynchronous SPI.

Can I merge this?

manovotn commented 9 months ago

Just a recommendation as it cannot be tested at this point. Even in the future, detail would need to be specified for both Java SE and EE environment as the later should have some integration with concurrency in the absence of a common asynchronous SPI.

Can I merge this?

I am +1 but would like to hear from @Azquelt before we merge it.

Ladicek commented 9 months ago

Testability is a good point. That's why I suggested we might want to turn the recommendation [of how the optional feature should work] into a requirement. There is currently a test that verifies that @Dependent beans are destroyed -- the target method in that test is synchronous, but one might argue that someone might want to consider it asynchronous. If that happened, the test would still be valid if the implementation followed the recommendation. If we turned the recommendation into a requirement, the test would always be valid.