Re: [weld-dev] AfterBeanDiscovery and observer resolver cache

Dne 6.3.2014 15:51, Christian Sadilek napsal(a): > Hi guys, > > Fair enough. This is the outcome I expected. I do think, however, that > the docs for ABD need some clarification here and ideally the ABD > instance should be "deactivated" once the bootstrap process finished so > it can throw an exception when e.g. addObserverMethod is invoked too > late in the game (better than silently having no effect). +1 I've created https://issues.jboss.org/browse/CDI-425 to address this issue. > > Thanks for your answers! > > Cheers, > Christian > > On 2014-03-06, at 5:24 AM, Jozef Hartinger <jharting(a)redhat.com > <mailto:jharting@redhat.com>> wrote: > >> Yes, I don't think Errai should depend on this behavior as it is not >> even in the "undefined" category but rather in "implicitly not >> allowed" or "not the intention of the spec" category. >> >> Even from the Weld point of view we use two levels of caching and >> while clearing the main one is easy, dealing with the second layer >> would require adding further complexity just to support this corner case. >> >> Therefore, I would suggest to use the other approach you proposed >> where you register a general observer method and dispatch to the >> dynamically added observers within there. It is always hard to guess >> performance implications without doing measurements but as long as the >> general observer method implementation is efficient, I would not worry >> about that one additional method invocation much. >> >> Jozef >> >> On 03/06/2014 10:04 AM, Mark Struberg wrote: >>> Hi Christian! >>> >>> While I find it nice that this works with OWB I also have to agree >>> that this is not a guaranteed behaviour by the spec intention. >>> What you could do in to hack around this issue is to have an >>> @Observes@Any Object method which delivers the events with your own >>> dynamic rules. >>> But be prepared that this might slow down your app a bit. >>> >>> LieGrue, >>> strub >>> >>> >>> >>> On Wednesday, 5 March 2014, 17:04, Christian Sadilek >>> <csadilek(a)redhat.com&gt; wrote: >>> >>> Hi, >>> >>> Yes, I expected this to not be an officially supported use case. >>> So, I guess it's just a matter of improving the API >>> design/documentation. >>> >>> However, dynamically registering an observer method would really >>> be useful in the case of Errai where we set up an event bridge >>> between the server and the client and potentially discover new >>> observers at runtime. >>> >>> We could work around this by registering an observer method that >>> observes all events and then handle the dispatching internally >>> but that seems less efficient. Right now this works because >>> OpenWebBeans doesn't cache the observers and allows invocations >>> to AfterBeanDiscovery.addObserverMethod at runtime and because >>> Weld has the functionality to clear the observer cache (although >>> that's not public API). >>> >>> My questions is: Is there a good reason not to support this going >>> forward? Can we add alternative functionality to add observer >>> methods at runtime (not using ABD)? >>> >>> Cheers, >>> Christian >>> >>> On 2014-03-05, at 4:37 AM, Martin Kouba <mkouba(a)redhat.com >>> <mailto:mkouba@redhat.com>> wrote: >>> >>>> FYI I've informed CDI EG and it will be discussed on the next >>> meeting >>>> whether to clarify this already in CDI 1.2 MR... >>>> >>>> M >>>> >>>> Dne 5.3.2014 10:19, Jozef Hartinger napsal(a): >>>>> Agreed. It is definitely not the intention of the >>> specification to allow >>>>> beans/observers/contexts to be added at runtime and >>> applications should >>>>> not have any expectations of what these methods do when >>> invoked outside >>>>> of the AfterBeanDiscovery observer. >>>>> >>>>> We'll add stricter validation to Weld to disallow this. >>>>> >>>>> On 03/05/2014 08:53 AM, Martin Kouba wrote: >>>>>> Hi Christian,this >>>>>> >>>>>> actually invoking any container lifecycle event method after the >>>>>> container initialization finished should have no effect. ABD >>> event >>>>>> reference can escape but it does not mean you can invoke >>> ABD.addBean() >>>>>> after ADV is fired. >>>>>> >>>>>> The spec wording is not very explicit here: >>>>>> "During the application initialization process, the container >>> fires a >>>>>> series of events, allowing portable extensions to integrate with >>>>>> the container initialization process defined in Section 12.2." >>>>>> >>>>>> Maybe we should file a new spec issue to clarify that such >>> invocations >>>>>> should result in IllegalStateException... >>>>>> >>>>>> Martin >>>>>> >>>>>> >>>>>> Dne 4.3.2014 17:42, Christian Sadilek napsal(a): >>>>>>> Hi Jozef, >>>>>>> >>>>>>> I think clearing the cache at the end of the Weld bootstrap >>> process is not enough to solve that particular problem since a >>> CDI extension can hold on to the ABD reference and invoke >>> addObserverMethod later (multiple times) which causes the same >>> problem I described below. There's no indication to the caller of >>> addObserverMethod that it's in fact too late to call that method. >>>>>>> >>>>>>> Since an ABD event reference can always escape (can be used >>> outside the method that observes it) it seems this issue should >>> be resolved (although it admittedly is an edge case). >>>>>>> >>>>>>> Cheers, >>>>>>> Christian >>>>>>> >>>>>>> On 2014-03-04, at 11:29 AM, Jozef Hartinger >>> <jharting(a)redhat.com <mailto:jharting@redhat.com>> wrote: >>>>>>> >>>>>>>> Hi Christian, >>>>>>>> >>>>>>>> this sounds like a bug. All the resolution caches should be >>> cleared at the very end of Weld's bootstrap sequence (after ABD >>> observers are called). (see >>> https://github.com/weld/core/blob/master/impl/src/main/java/org/jboss/wel...) >>>>>>>> >>>>>>>> Jozef >>>>>>>> >>>>>>>> On 03/04/2014 04:36 PM, Christian Sadilek wrote: >>>>>>>>> Hi everyone, >>>>>>>>> >>>>>>>>> CDI extensions can observe the AfterBeanDiscovery event to >>> register observer methods (addObserverMethod). However, when an >>> event is first fired, the observers for that event are resolved >>> and then cached (in TypeSafeResolver). All future calls to >>> addObserverMethod for an already fired event with corresponding >>> qualifiers will have no effect because the observer result is >>> read from cache and not recomputed. >>>>>>>>> >>>>>>>>>> From an API perspective that's unfortunate because >>> addObserverMethod will only work until an event (with >>> corresponding qualifiers) is fired and there is no indication to >>> the caller of that method that it didn't have any effect when >>> invoked after that. >>>>>>>>> >>>>>>>>> Possible solutions: >>>>>>>>> >>>>>>>>> - Provide some public API to clear/recompute that part the >>> observer cache. Maybe that exists? I couldn't find it which is >>> why I am using the private API and Reflection :(. Also let >>> AfterBeanDiscovery.addObserverMethod fail in that case with the >>> advice to reset the cache. >>>>>>>>> >>>>>>>>> - Recompute the corresponding part of the cache when >>> addObserverMethod is called (seems preferable). >>>>>>>>> >>>>>>>>> OpenWebBeans doesn't have this issue as their >>> NotificationManager will simply add the new ObserverMethod to a >>> ConcurrentHashMap that is also accessed when an event is fired. >>>>>>>>> >>>>>>>>> What do you think? Can this already be done or is there >>> another solution? >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> Christian >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> weld-dev mailing list >>>>>>>>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>>>> >>>>>>> _______________________________________________ >>>>>>> weld-dev mailing list >>>>>>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>>>>>> https://lists.jboss.org/mailman/listinfo/weld-dev >>>>>>> >>>>> >>> >>> >>> _______________________________________________ >>> weld-dev mailing list >>> weld-dev(a)lists.jboss.org <mailto:weld-dev@lists.jboss.org> >>> https://lists.jboss.org/mailman/listinfo/weld-dev >>> >>> >>> >>> >>> _______________________________________________ >>> weld-dev mailing list >>> weld-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/weld-dev >> >