Allow relaxing invoker lookup rules for asynchronous methods

[Ladicek]

No description provided.

[Ladicek]

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]

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]

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]

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]

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.

[Azquelt]

Here are my thoughts:

1. This text doesn't require or permit anything, so we could leave it out altogether. Implementations are always free to provide additional APIs and configuration options to extend the spec behaviour.

2. I'm not sure who we mean by "integrator" here. Is that the writer of the extension which creates the invoker? Or do we mean someone integrating a CDI implementation into their application server?
   I think it's the former? In that case I think the responsibilities are:

   • the CDI implementation makes an API available to destroy looked up @Dependent beans
   • the user ensures it is called at the correct time

   In that case, the recommendation of when to destroy looked up @Dependent beans is guidance to the user (likely a library author), not to the implementation writer. This means that it could never become a requirement, since we can't require users to do things, but it does provide guidance on the things that the implementor-provided API should allow, or for any integrations that might be provided out of the box (e.g. for CompletionStage).
   In a future release, we might specify the API for doing this, but I don't know that we need to mention that.

With those thoughts in mind, here are some proposed changes:

Suggested change

	This specification recognizes the existence of _asynchronous_ methods, where the action represented by the method does not always finish when the method returns; the _completion_ of the action is asynchronous to the method call.
	Implementations are permitted (but not required) to provide an API that allows integrators to specify which target methods shall be considered asynchronous and how completion is propagated to the caller.
	For asynchronous target methods, the requirement to destroy instances of `@Dependent` looked up beans is relaxed: the instances of `@Dependent` looked up beans need not be destroyed before `Invoker.invoke()` returns, it is instead the responsibility of the implementation and the integrator to ensure that they are destroyed at the right moment.
	It is recommended that the instances of `@Dependent` looked up beans are destroyed after the asynchronous action completes and before the completion is propagated to the caller of `Invoker.invoke()`; if an asynchronous target method completes synchronously or throws synchronously, it is recommended that the instances of `@Dependent` looked up beans are destroyed before `invoke()` returns or rethrows the exception.
	NOTE: This optional behavior may become a requirement in a future version of this specification.
	This specification recognizes the existence of _asynchronous_ methods, where the action represented by the method does not always finish when the method returns; the _completion_ of the action is asynchronous to the method call. For these methods, it may be desirable to defer the destruction of any `@Dependent` looked up beans until the asynchronous action completes.
	Implementations are permitted (but not required) to provide an API to specify which target methods shall be considered asynchronous and to control when looked up `@Dependent` beans are destroyed for invocations of those methods.
	As a general rule, it is recommended that the instances of looked up `@Dependent` beans are destroyed after the asynchronous action completes and before the completion is propagated to the caller of `Invoker.invoke()`; if an asynchronous target method completes synchronously or throws synchronously, it is recommended that the instances of looked up `@Dependent` beans are destroyed before `invoke()` returns or rethrows the exception.

[Ladicek]

By integrator, I mean whoever takes the CDI implementation (such as Weld) and makes it part of the cohesive whole that runs an application. That would typically be an application server or framework, but in case of standalone usage (CDI SE), that would be the end user. I didn't mean the author of a hypothetical CDI extension that creates an invoker.

I guess there's a better term for this, I just don't know it.

since we can't require users to do things

Sure we can :-) All definition errors and deployment problems are signs of users not upholding the CDI requirements. (Technically, this is us requiring users to not do things. I don't think there's a huge difference, but I could probably be convinced otherwise.)

[Azquelt]

since we can't require users to do things

Sure we can :-) All definition errors and deployment problems are signs of users not upholding the CDI requirements. (Technically, this is us requiring users to not do things. I don't think there's a huge difference, but I could probably be convinced otherwise.)

Ok, that's true :-) we definitely do place requirements on users if they want things to work, but here it sounded like we're saying "here's an API and here's what you must implement with it", whereas generally we don't say the second bit, we either state what the API will do and allow the user to use it for whatever purpose, or we give the user an interface to implement which describes the expected contract.

I guess it's not that important a distinction.

By integrator, I mean whoever takes the CDI implementation (such as Weld) and makes it part of the cohesive whole that runs an application. That would typically be an application server or framework, but in case of standalone usage (CDI SE), that would be the end user. I didn't mean the author of a hypothetical CDI extension that creates an invoker.

I guess there's a better term for this, I just don't know it.

I don't think the spec differentiates between these two roles. 1.1 only lists two parties that the spec assigns responsibilities for. I'm not aware of anywhere else where it defines a contract between an implementor and an integrator. That sort of thing is generally left up to them to define between themselves (e.g. as weld does).

Implementations are permitted (but not required) to provide an API that allows integrators (as defined above) to specify which target methods shall be considered asynchronous and how completion is propagated to the caller.

This I disagree on. This only makes sense in the narrow use case where the integrator is internally using method invokers to integrate with some other part of the runtime and has knowledge of where other parts of the runtime use asynchronous methods.

If the intent is to allow internal extensions for use within the runtime, then I don't think we need to mention that in the specification because it shouldn't be visible to the user.

If it is visible to the user because we want to allow the container to use a general rule like "methods which return CompletionStage are asynchronous", then I'd specify it like this:

Suggested change

	This specification recognizes the existence of _asynchronous_ methods, where the action represented by the method does not always finish when the method returns; the _completion_ of the action is asynchronous to the method call.
	Implementations are permitted (but not required) to provide an API that allows integrators to specify which target methods shall be considered asynchronous and how completion is propagated to the caller.
	For asynchronous target methods, the requirement to destroy instances of `@Dependent` looked up beans is relaxed: the instances of `@Dependent` looked up beans need not be destroyed before `Invoker.invoke()` returns, it is instead the responsibility of the implementation and the integrator to ensure that they are destroyed at the right moment.
	It is recommended that the instances of `@Dependent` looked up beans are destroyed after the asynchronous action completes and before the completion is propagated to the caller of `Invoker.invoke()`; if an asynchronous target method completes synchronously or throws synchronously, it is recommended that the instances of `@Dependent` looked up beans are destroyed before `invoke()` returns or rethrows the exception.
	NOTE: This optional behavior may become a requirement in a future version of this specification.
	This specification recognizes the existence of _asynchronous_ methods, where the action represented by the method does not always finish when the method returns; the _completion_ of the action is asynchronous to the method call.
	For target methods the container deems to be asynchronous, the requirement to destroy instances of `@Dependent` looked up beans is relaxed: the instances of `@Dependent` looked up beans need not be destroyed before `Invoker.invoke()` returns, it is instead the responsibility of the container to ensure that they are destroyed at the right moment.
	It is recommended that the instances of `@Dependent` looked up beans are destroyed after the asynchronous action completes and before the completion is propagated to the caller of `Invoker.invoke()`; if an asynchronous target method completes synchronously or throws synchronously, it is recommended that the instances of `@Dependent` looked up beans are destroyed before `invoke()` returns or rethrows the exception.
	CAUTION: the rules for how the container deems a method to be asynchronous are not defined and so applications which use invokers to call asynchronous methods are not portable. Future versions of this specification may define an API to give greater control over the invocation of asynchronous methods.

This is clearer that the behaviour is not defined and therefore not portable. There's no mention of providing an API, since we're not providing an API to the user.

I don't like defining it this way, but if that's what everyone is in favour of, then it needs to be clear that it's not portable.

[Ladicek]

That sounds like a good way to phrase it. I agree that we don't need to mention that an integration API may exist, as that's indeed a concern of the implementation, not the specification. I'll sleep over it and then probably adopt this wording.

[Ladicek]

Done. I ended up changing the text a little, but should have the same meaning.