jakartaee / concurrency

Eclipse Project for Concurrency Utilities
https://projects.eclipse.org/projects/ee4j.cu
Other
70 stars 39 forks source link

Concurrency (JSR-236) is problematic in practice. Pls rework the API and fully support CDI. #105

Open smillidge opened 4 years ago

smillidge commented 4 years ago

Copied from https://github.com/jakartaee/specifications/issues/129 raised by @Tibor17

I reported issues against the Wildfly due to this API is not practical in commercial EE world but the issues were closed due to the problem is here in the Spec. The developer wants to execute Task as a Bean in the ThreadPool. If you have a producer of EntityManager, these Task beans won't see it! You won't be able to use the scopes because they do not have the end in the Thread and therefore the JDBC connection is permanently open. Pls support using CDI beans and scopes in the concurrent Task beans without any additional need to construct them via Proxy, etc.

I like to have JSR 236, that's why my company used it, but in another fashion, I would say, and in more EE Beans Context friendly style which would speedup my s/w development of the projects.

Perhaps the dev community should re-evaluate the classes in javax.enterprise.concurrent, see 1, and differentiate between internal needs (for e.g. Async in JAX-RS and application servers) and the external needs in the users community. Me as a user expected this API in JSR 236 to fulfil my needs in order to schedule Tasks in ManagedScheduledExecutorService instead of doing it in EJB Timer, of course the ThreadPool should understand the CDI context because my company is using the CDI.

As i said before I have problem with the way how the beans (via proxy) in JSR 236 are used in terms of beans context. I am still pointing to the practicle use.

The JSR 236 was found as a substitution of EJB Timer but again it had problems with Beans Context, strange Proxying style, strange way to enable transactions, etc. It would be enough to say in the spec that the Managed*ExecutorService should be able to work with CDI beans with Application scope and Request scope and then Jakarta EE can remove ContextService, "executionProperties". Internally, the application server may still have to use the ManagedExecutorService (not appear in JavaDoc) but the enduser may prefer (should be in JavaDoc) another API, something like this:

@RequestScoped
public class ShortLifecycleBean implements Runnable {
  @Inject EntityManager em;

  public void run() { ... }
}

@Resource
ManagedScheduler<ShortLifecycleBean> scheduler;

// NOTICE: the Runnable is not in the first parameter because the ManagedScheduler
// will "call us and we will NOT pass our bean to ManagedScheduler" => IoC
Future<?> f = scheduler.schedule( trigger );

Therefore the new interface ManagedScheduler is the manager (IoC) of the beans again!

njr-11 commented 4 years ago

Thanks for reporting this and offering the suggestions for improvement. I think this should be split into several different issues, which can all be considered separately.

The biggest request here is the support for CDI context. There is actually an issue https://github.com/eclipse-ee4j/concurrency-api/issues/39 already open for that. This will require a lot of collaboration with the CDI spec group, as I indicated in a comment to that issue, the CDI spec really needs to define what it means for CDI contexts to be captured and re-established and levy the necessary requirements against CDI implementations in order for an implementation of the Concurrency spec to be able to accomplish it. Fortunately, Weld (a CDI implementation) and MicroProfile Context Propagation (which has been experimenting with innovations within the concurrency space while work on the EE specs was paused) have prototyped an implementation of this, which could become the basis of a solution. @Tibor17 given your interest in this area, it would be great if you could try out what was done in MicroProfile and Weld and offer feedback on whether you feel that has taken the right direction or if there are shortcomings. If you want to try it out -- In MicroProfile, your application uses a special builder to configure and create a ManagedExecutor instance (this is where you tell it that you want CDI context included), and in at least one of the implementations, you can then cast the resulting instance directly to the EE Concurrency ManagedExecutorService interface and use it that way if you wish to otherwise avoid MicroProfile interfaces. It should be noted that MicroProfile doesn't have the schedule operation to experiment with, but I don't see any reason why it would be different from submit/execute, just running at a scheduled time further into the future.

The second request being made here is for removal of ContextService and executionProperties. For the sake of compatibility with existing applications, I don't see anything being removed at least in the near term, but certainly deprecating and de-emphasizing could be appropriate, especially if we end up with alternatives such as are proposed in other issues.

The third request being made here is for the introduction of a new ManagedScheduler resource that is paired with a particular Runnable implementation. My own thoughts on this is that it seems like too much magic that makes code more difficult to follow, and I'd prefer to just continue to specify the Runnable/Callable and write code that is more explicit. Although I also recognize I may have a minority view point on those sorts of patterns, because the trend does seem to be in the direction of less explicit code and more injection magic. This deserves more discussion.