Container managed executors and `shutdown`

[FroMage]

The javadoc for ManagedExecutor says:

 * <p>Managed executors that are created with the {@link Builder} or created for
 * injection into applications via CDI must support the life cycle methods, including:
 * awaitTermination, isShutdown, isTerminated, shutdown, shutdownNow.
 * The application must invoke <code>shutdown</code> or <code>shutdownNow</code> to terminate
 * the life cycle of each managed executor that it creates, once the managed executor is
 * no longer needed. When the application stops, the container invokes <code>shutdownNow</code>
 * for any remaining managed executors that the application did not already shut down.
 * The shutdown methods signal the managed executor that it does not need to remain active to
 * service subsequent requests and allow the container to properly clean up associated resources.</p>
 *
 * <p>Managed executors which have a life cycle that is scoped to the container,
 * including those obtained via mechanisms defined by EE Concurrency, must raise
 * IllegalStateException upon invocation of the aforementioned life cycle methods,
 * in order to preserve compatibility with that specification.</p>

@manovotn noted that we do not test this in the TCK, so it's not entirely clear if this would be acceptable behaviour.

In Quarkus we're using an executor "view" on container-managed executors, so as to be able to limit submissions with maxAsync and maxQueued over any container-managed thread pool we do not create. Here, of course we do not want shutdown on our ManagedExecutor to shutdown the container-managed executor, but the view we have gives us an alternative, which is to shutdown the view itself, which may have some state and queues.

So it's not entirely clear if we should say that ManagedExecutor backed by container-managed executors should throw on shutdown or not: it could be useful to support it, or silently fail to propagate to the underlying executor if it's container-managed.

WDYT? Is this something that other implementations of the container view executor would benefit from?

[njr-11]

Hopefully I can help clarify this, and then we can figure out whether an improvement is needed to the wording used by the specification. We, too, have an implementation where each ManagedExecutor is not a thread pool of its own, but is instead backed by a container managed thread pool, and we are passing the TCK. The spec was written to fully allow for implementers to make this choice in their implementations.

First, I'll note that the JavaDoc for shutdown and shutdownNow on java.util.concurrent.ExecutorService is written solely in terms of rejecting new tasks and canceling existing tasks. In no way does it state or imply any requirement of destroying of underlying threads or thread pools. The MicroProfile Context Propagation TCK currently includes 2 tests (one for shutdown, and another for shutdownNow) validating implementations in this area.

org.eclipse.microprofile.context.tck.ManagedExecutorTest.shutdownNowPreventsAdditionalSubmitsAndCancelsTasks()
org.eclipse.microprofile.context.tck.ManagedExecutorTest.shutdownPreventsAdditionalSubmits()

These TCK tests require that all implementations of ManagedExecutor that are obtained from the Builder (which is the only mechanism defined by the MP Context Propagation Spec) must support shutdown/shutdownNow, which is to be naturally expected due to the inheritance hierarchy where ManagedExecutor extends java.util.concurrent.ExecutorService. The MP Context Propagation spec further clarifies this in the ManagedExecutor JavaDoc that you cited.

The ManagedExecutor JavaDoc also discusses "Managed executors which have a life cycle that is scoped to the container". The MP Context Propagation spec does not define any way to create such "Managed executors". The EE Concurrency spec however, does define a way, and also forbids implementation of the shutdown/shutdownNow methods. This language reconciles the two specs, for the sake of implementations that implement both specs. If the EE Concurrency mechanism is used, you get a managed executor scoped to the container life cycle that abides by the EE Concurrency requirement to disallow shutdown/shutdownNow. If the MP Context Propagation mechanism is used, you get a managed executor whose life cycle is managed by the application which abides by the MP Context Propagation requirement to provide shutdown/shutdownNow.

If you are concerned that the language in the JavaDoc is telling you what you can and cannot do within product internals (container-managed thread pool that is backing managed executor implementations), you should not be concerned about this. JavaDoc requirements apply specifically to the org.eclipse.microprofile.context.ManagedExecutor or javax.enterprise.concurrent.ManagedExecutorService that is supplied to the end user, not to any generic concept of "managed executors" that might exist within product internals.

[njr-11]

To address the question more succinctly:

So it's not entirely clear if we should say that ManagedExecutor backed by container-managed executors should throw on shutdown or not

The following ought to be clear from the JavaDoc: If the aforementioned ManagedExecutor came from the ManagedExecutor.Builder, then it must implement shutdown and shutdownNow, as enforced by the TCK. If the aforementioned ManagedExecutor was created by the container via other means (such as EE Concurrency) and scoped to the container's life cycle, then shutdown/shutdownNow must raise IllegalStateException as required by the EE Concurrency spec. Whether it is backed by a container-managed thread pool/executors is not relevant.

[FroMage]

OK. I get it now, and I agree the current wording is enough, and we can point to this issue if anyone asks for more details. Thanks!