Re: [weld-dev] Should EventImpl.fireAsync use ExecutorServices from the ServiceRegistry

2018-07-18 Thread Benjamin Confino
Hi Martin. 

My mistake. I thought the EventImpl was ignoring 
ExecutorServices#getTaskExecutor() but after reading your email I checked 
again and found it later down the stack, and upon further investigation 
the Executor we're providing doesn't handle what I was told it would, 
which means it can be fixed. Thanks for your help. 

P.S. The NPE is CDI.current() returning null, then a method is called on 
that null. It's the result of some ThreadLocals not being set inside our 
code.



From:   Martin Kouba 
To: Benjamin Confino , weld-dev@lists.jboss.org
Cc: Tom Evans , Emily Jiang 
Date:   18/07/2018 08:18
Subject:Re: [weld-dev] Should EventImpl.fireAsync use 
ExecutorServices from the ServiceRegistry



Hi Benjamin,

If no Executor is provided - either fireAsync(U event) or no executor 
set to the notification options - Weld uses the Executor returned from 
org.jboss.weld.manager.api.ExecutorServices#getTaskExecutor() or 
ForkJoinPool#commonPool() if no ExecutorServices is registered. In other 
words, it's up to the integrator which Executor is used.

And yes, Event#fireAsync(U, NotificationOptions) javadoc is outdated - 
in the first versions it was only possible to pass an Executor.

What does the NPE look like?

Thanks,

Martin

Dne 17.7.2018 v 18:30 Benjamin Confino napsal(a):
> Hello
> 
> I have a customer with the following
> 
> @Inject
>  Event event;
> 
>  @Resource
>  ManagedExecutorService threadPool;
> 
> public void fireAsyncEvent() {
>System.out.println("Sending Async-Message via CDI");
>event.fireAsync(new Message("Hello"));
>  }
> 
>  public void fireAsyncEvent_2() {
>event.fireAsync(new Message("Hello"), 
> NotificationOptions.ofExecutor(threadPool));
>  }
> 
> fireAsyncEvent() eventually results in an NPE when a JSF class attempts 
> tries to call CDI.current() fireAsyncEvent_2() works correctly. I was 
> hoping that it would be possible to fix fireAsyncEvent to remove the 
> dependency on passing in an executor service every time it is used.
> 
> We currently set an executor service into the weld service registry, and 

> I was wondering if org.jboss.weld.event.EventImpl fetch the  executor 
> service from the service registry. Either only when fireAsync is called 
> without any NotificationOptions, or perhaps more aggressively and set 
> the default executor service if NotificationOptions are provided but do 
> not explicitly contain an executor service (The javadoc is unclear, but 
> I believe calling ofExecutor() provides this functionality).
> 
> Before going further with this I wanted to ask for a quick sanity check. 

> Is this fix plausible?
> 
> Regards
> Benjamin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 

> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
> 
> 
> ___
> weld-dev mailing list
> weld-dev@lists.jboss.org
> 
https://lists.jboss.org/mailman/listinfo/weld-dev

> 

-- 
Martin Kouba
Senior Software Engineer
Red Hat, Czech Republic




Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
___
weld-dev mailing list
weld-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/weld-dev

Re: [weld-dev] Should EventImpl.fireAsync use ExecutorServices from the ServiceRegistry

2018-07-18 Thread Martin Kouba
Hi Benjamin,

If no Executor is provided - either fireAsync(U event) or no executor 
set to the notification options - Weld uses the Executor returned from 
org.jboss.weld.manager.api.ExecutorServices#getTaskExecutor() or 
ForkJoinPool#commonPool() if no ExecutorServices is registered. In other 
words, it's up to the integrator which Executor is used.

And yes, Event#fireAsync(U, NotificationOptions) javadoc is outdated - 
in the first versions it was only possible to pass an Executor.

What does the NPE look like?

Thanks,

Martin

Dne 17.7.2018 v 18:30 Benjamin Confino napsal(a):
> Hello
> 
> I have a customer with the following
> 
>         @Inject
>          Event event;
> 
>          @Resource
>          ManagedExecutorService threadPool;
> 
>         public void fireAsyncEvent() {
>        System.out.println("Sending Async-Message via CDI");
>        event.fireAsync(new Message("Hello"));
>          }
> 
>          public void fireAsyncEvent_2() {
>        event.fireAsync(new Message("Hello"), 
> NotificationOptions.ofExecutor(threadPool));
>          }
> 
> fireAsyncEvent() eventually results in an NPE when a JSF class attempts 
> tries to call CDI.current() fireAsyncEvent_2() works correctly. I was 
> hoping that it would be possible to fix fireAsyncEvent to remove the 
> dependency on passing in an executor service every time it is used.
> 
> We currently set an executor service into the weld service registry, and 
> I was wondering if org.jboss.weld.event.EventImpl fetch the  executor 
> service from the service registry. Either only when fireAsync is called 
> without any NotificationOptions, or perhaps more aggressively and set 
> the default executor service if NotificationOptions are provided but do 
> not explicitly contain an executor service (The javadoc is unclear, but 
> I believe calling ofExecutor() provides this functionality).
> 
> Before going further with this I wanted to ask for a quick sanity check. 
> Is this fix plausible?
> 
> Regards
> Benjamin
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> 
> 
> ___
> weld-dev mailing list
> weld-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/weld-dev
> 

-- 
Martin Kouba
Senior Software Engineer
Red Hat, Czech Republic
___
weld-dev mailing list
weld-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/weld-dev