Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reactive CompletionStage backed by ManagedExecutorService #40

Closed
glassfishrobot opened this issue Dec 15, 2017 · 7 comments · Fixed by #104
Closed

Reactive CompletionStage backed by ManagedExecutorService #40

glassfishrobot opened this issue Dec 15, 2017 · 7 comments · Fixed by #104
Assignees
Labels
enhancement New feature or request microprofile For compatibility with MicroProfile Type: New Feature
Milestone

Comments

@glassfishrobot
Copy link

There is a discussion in the JAX-RS expert group list (jaxrs-spec@javaee.groups.io : Clarification regarding ManagedExecutorService in Rx Client) regarding the use of managed executors for the JAX-RS reactive client. Specifically, it has been pointed out that while the JAX-RS 2.1 spec requires the ability to create a CompletionStage with a ManagedExecutorService as its default asynchronous execution facility, there is currently no means for a JAX-RS provider to implement this requirement such that it behaves in a reliable way with respect to thread context propagation.

This situation arises because CompletionStage async methods that add dependent stages are sometimes a deferred submit of the task and sometimes not, depending on whether or not the parent stage happens to have already completed. If the parent stage is already completed, the submit is not deferred, and thread context can naturally be captured from the thread that adds the dependent stage, as would be expected. However, if the parent stage has not completed, the submit is deferred until such point in time as the parent stage completes, after which the task to run the dependent stage is submitted, however, inheriting the wrong thread context - the context under which the parent stage ran.

The non-deterministic behavior is not only undesirable, it is a potential security issue because any code with access to the CompletionStage can append additional work in the form of a dependent stage that will, provided the parent stage hasn't completed yet, be able to run under the prior action's security context.

Solving this problem will require a CompletionStage with awareness of managed executors which can more properly interface with them. Given the applicability of this same issue beyond JAX-RS (this pattern can arise in general usage as well), I'm recommending that we try to solve it within the EE Concurrency Utilities spec, by adding the ability on ManagedExecutorService to create CompletionStages that are backed by the ManagedExecutorService as the default asynchronous execution facility and which propagate thread context in a well-defined manner: by always capturing it from the thread that creates the CompletionStage, whether created as the initial stage from the ManagedExecutorService or created as a dependent stage.

I'd start out by proposing that ManagedExecutorService be given equivalents to the static methods that CompletableFuture has (except as non-static for ManagedExecutorService):

/**
 * Returns a new CompletionStage that is completed by a task running in this
 * ManagedExecutorService after it runs the given action.
 * This ManagedExecutorService is the default asynchronous execution facility for the
 * new CompletionStage and all dependent CompletionStages that are created from it,
 * and all dependent CompletionStages that are created from those, and so forth.
 * Thread context is captured from the thread that creates the CompletionStage and is
 * applied to the thread that runs the task, being removed afterward.
 * When dependent CompletionStage instances are created from the CompletionStage,
 * and so forth from any dependent stage, thread context is captured from the thread
 * that creates each dependent stage.  This guarantees that the action of a CompletionStage
 * always runs under the thread context of the code that creates the stage. 
 *
 * @param runnable the action to run before completing the returned CompletionStage
 * @return the new CompletionStage
 */
public CompletionStage<Void> runAsync(Runnable runnable);

/**
 * Returns a new CompletionStage that is completed by a task running in this
 * ManagedExecutorService with the value obtained by calling the given Supplier.
 * This ManagedExecutorService is the default asynchronous execution facility for the
 * new CompletionStage and all dependent CompletionStages that are created from it,
 * and all dependent CompletionStages that are created from those, and so forth.
 * Thread context is captured from the thread that creates the CompletionStage and is
 * applied to the thread that runs the task, being removed afterward.
 * When dependent CompletionStage instances are created from the CompletionStage,
 * and so forth from any dependent stage, thread context is captured from the thread
 * that creates each dependent stage.  This guarantees that the action of a CompletionStage
 * always runs under the thread context of the code that creates the stage. 
 *
 * @param supplier a function returning the value to be used to complete the returned CompletionStage
 * @return the new CompletionStage 
 */
public <U> CompletionStage<U> supplyAsync(Supplier<U> supplier);

/**
 * Returns a new CompletionStage that is already completed with the given value.
 * This ManagedExecutorService is the default asynchronous execution facility for the
 * new CompletionStage and all dependent CompletionStages that are created from it,
 * and all dependent CompletionStages that are created from those, and so forth.
 * When dependent CompletionStage instances are created from the CompletionStage,
 * and so forth from any dependent stage, thread context is captured from the thread
 * that creates each dependent stage.  This guarantees that the action of a CompletionStage
 * always runs under the thread context of the code that creates the stage. 
 *
 * @param value the value
 * @return the completed CompletionStage
 */
public <U> CompletionStage<U> completedStage(U value);

Having just finished writing this up, I checked the thread in the JAX-RS expert group again, and there is also the suggestion that EE Concurrency spec could provide a ManagedCompetableFuture, which is also an excellent suggestion. It's basically the same as above except that the methods would be static, accept a ManagedExecutorService as an argument, and return ManagedCompletableFuture. Either of these solutions would solve the problem.

@glassfishrobot
Copy link
Author

@mkarg Commented
Thanks @njr-11 for filing this issue. I am the originator of the counterproposal (ManagedCompletableFuture). Having a separate class ManagedCompletableFuture would allow to further customize it, if that need arises. Also, there is no need to make it aware of the ManagedExecutorService at all, as its actual "magic" would be specific to the provider of the concur spec implementation. Most roughly spoke, all the spec needs to mandate would be that it captures context in the same way as ManagedExecutorService, it uses that executor if no other explicitly provided, and it captures context at time of task provision instead of task submission.

@glassfishrobot
Copy link
Author

@njr-11
Copy link
Contributor

njr-11 commented Nov 9, 2018

I should point out that a solution to this is being prototyped under the MicroProfile Concurrency spec (MicroProfile FaultTolerance spec requires this function), with the hope that it will be brought back into Jakarta EE Concurrency once new releases of EE specs are ready to start incorporating enhancements.

@smillidge
Copy link
Contributor

I think we should start gathering some requirements for a next version of concurrency utils and start a new release plan.

@njr-11
Copy link
Contributor

njr-11 commented Nov 12, 2018

@smillidge agreed. A couple of good sources of requirements will be the various issues opened against this repo, as well as what has gone into MicroProfile Concurrency. I assume the cu-dev mailing list will be the best place for the discussion.

@smillidge smillidge added enhancement New feature or request Type: New Feature labels Sep 13, 2019
@njr-11
Copy link
Contributor

njr-11 commented Nov 13, 2019

Once we are allowed to start on Jakarta EE 10 work (some time after the package rename occurs for Jakarta EE 9), I'll volunteer to put together a pull for this issue that copies over the following interface methods from the MicroProfile managed executor, all of which parallel java.util.concurrent.CompletableFuture's static methods and newIncompleteFuture method for creating completable futures/completion stages,

<U> CompletableFuture<U> completedFuture(U value);
<U> CompletionStage<U> completedStage(U value);
<U> CompletableFuture<U> failedFuture(Throwable ex);
<U> CompletionStage<U> failedStage(Throwable ex);
<U> CompletableFuture<U> newIncompleteFuture();
CompletableFuture<Void> runAsync(Runnable runnable);
<U> CompletableFuture<U> supplyAsync(Supplier<U> supplier);

This, in addition to copying over some JavaDoc at the class level that defines what it means for a completion stage to be backed by a managed executor, should get Jakarta EE up to the point of matching, in a fully compatible way, what Microprofile provides around managed executor backed completion stages.

All of the other MicroProfile Context Propagation enhancements/features can be covered in separate issues, which will allow the Jakarta EE Concurrency participants to decide on a granular basis what to accept into Jakarta EE and what not to.

@njr-11 njr-11 self-assigned this Nov 13, 2019
@njr-11 njr-11 added the microprofile For compatibility with MicroProfile label Nov 13, 2019
@smillidge
Copy link
Contributor

We are allowed to do Jakarta EE 10 work now. Maybe in the first instance merge the namespace change to the 2.0 branch and then branch off that.

njr-11 added a commit to njr-11/concurrency-api that referenced this issue Nov 22, 2019
Signed-off-by: Nathan Rauh <nathan.rauh@us.ibm.com>
@smillidge smillidge added this to the 3.0 milestone Mar 11, 2021
njr-11 added a commit to njr-11/concurrency-api that referenced this issue Mar 17, 2021
njr-11 added a commit to njr-11/concurrency-api that referenced this issue Mar 17, 2021
Signed-off-by: Nathan Rauh <nathan.rauh@us.ibm.com>
njr-11 added a commit to njr-11/concurrency-api that referenced this issue Sep 15, 2021
Signed-off-by: Nathan Rauh <nathan.rauh@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request microprofile For compatibility with MicroProfile Type: New Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants