Thanks to everyone for consideration. > Yes, the Apache DeltaSpike project is the joined effort in this area. That's good to hear because CDI together with all the extensions already rocks and this may just make it even better.
> we imported it in deltaspike already -> please file an issue there [1]. Sure thing, on it. > + you are welcome to join the effort. I may need a mentor to get started since I've never had the privilege to contribute to any OSS project before. I promise to get out of your hair ASAP. Anyone willing to volunteer? :) Radu Creanga On Sun, Nov 18, 2012 at 1:54 PM, Gerhard Petracek <[email protected]> wrote: > hi radu, > > we imported it in deltaspike already -> please file an issue there [1]. > + you are welcome to join the effort. > > changes to the imported implementations will get reviewed and if it makes > sense we will merged them back to codi (esp. important fixes). > > regards, > gerhard > > [1] https://issues.apache.org/jira/browse/DELTASPIKE > > http://www.irian.at > > Your JSF/JavaEE powerhouse - > JavaEE Consulting, Development and > Courses in English and German > > Professional Support for Apache MyFaces > > > > > 2012/11/18 Mark Struberg <[email protected]> >> >> Hi Radu! >> >> Thansk for the feedback. Yes, observing session events would be a possible >> workaround for broken JSF containers/spec issues. >> >> > Did CODI join forces with Seam? >> Yes, the Apache DeltaSpike project is the joined effort in this area. For >> the @ViewScoped implementation this doesn't have any impact because I >> (together with the help Jakob and Gerhard) wrote both of them ;) >> >> >> >> LieGrue, >> strub >> >> >> >> ----- Original Message ----- >> > From: Radu Creanga <[email protected]> >> > To: MyFaces Development <[email protected]> >> > Cc: >> > Sent: Sunday, November 18, 2012 6:28 PM >> > Subject: Re: CODI: @ViewScoped implementation possibly flawed. >> > >> > Stan, >> > >> > I reviewed the spec changes. What they do is monitor session, session >> > attribute, context, context attributes, context, and context attribute >> > events then properly fire PreDestroyViewMapEvents. This change would >> > actually fix CODI's implementation as it stand since this is the event >> > that CODI's implementation monitors. >> > >> > Mark, >> > >> > "It actually has nothing to do with CODI nor CDI." - please allow me >> > to argue otherwise. The CDI spec says that Context implementations are >> > responsible for destroying beans, see here >> > (http://docs.jboss.org/cdi/spec/1.0/html_single/#contextual). Whoever >> > wrote the CODI implementation thought that listening for >> > PreDestroyViewMapEvents would be enough to ensure that, which I don't >> > blame them, look at the comments: >> > >> > /** >> > * We get PreDestroyViewMapEvent events from the JSF servlet and >> > destroy our contextual >> > * instances. This should (theoretically!) also get fired if the >> > webapp closes, so there >> > * should be no need to manually track all view scopes and destroy >> > them at a shutdown. >> > * >> > * @see >> > >> > javax.faces.event.SystemEventListener#processEvent(javax.faces.event.SystemEvent) >> > */ >> > >> > But, this event does not get fired, therefore a better implementation >> > would monitor for at least HttpSessionEvents and destroy the instances >> > it creates. >> > >> > Also, you say that it's a similar problem with every passivation >> > capable bean. Sure, in the case that the beans would be passivated or >> > replicated across the domain, I wouldn't blame the context for not >> > being able to destroy them. But, there are plenty of instances when >> > view scoped beans never get passivated, just like sessions. And the >> > current implementation doesn't even destroy those. >> > >> > Now, it is not possible (to the best of my knowledge) to register for >> > session events directly from the Contextual implementation, since by >> > then the servlet would be initialized. Therefore, writing a CDI >> > extension that propagates the servlet and session lifecycle events to >> > CDI, then having the Contextual implementation observe those events in >> > order to properly destroy instance it creates would be one solution. >> > >> > I understand that this may be overkill since the new JSF spec will fix >> > this in a few months. Also, from what I understand CODI does not >> > currently have an extension that monitors for servlet related events >> > so it would be quite a lot of work to get there. >> > >> > Did CODI join forces with Seam? If so, it may be worth bringing this >> > up to them. I'd be happy to work on it, since I already have to anyway >> > for my internal project. Pointers would be appreciated. >> > >> > Radu Creanga >> > >> > >> > On Fri, Nov 16, 2012 at 12:14 PM, <[email protected]> wrote: >> >> Problems with @ViewScoped and @PreDestroy have been identified in the >> >> spec and a fix is slated for JSF 2.2: >> >> http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-905 >> >> >> >> You guys might want to comment on it. >> >> >> >> Stan >> >> >> >> On 11/16/2012 9:50 AM, Mark Struberg wrote: >> >>> >> >>> Hi Radu! >> >>> >> >>> Congratulations, you spotted a very deeply buried problem :) It >> > actually has nothing to do with CODI nor CDI. The problem is similar >> > with every >> > passivation capable (==Serializable) bean: @PreDestroy is not 100% >> > guaranteed to >> > get fired. >> >>> >> >>> I'm not sure if you really did hit a bug in the 2 JSF impls or a >> > systematic consequence of serializable objects I like to explain now: >> >>> >> >>> >> >>> Think about a @ViewScoped bean. The @PreDestroy invokes a >> >>> user.logout() >> > for example. Now you navigate away from the page and the @ViewScoped >> > bean gets >> > closed. Then press the browser back button and you will have the bean >> > again >> > (restored from the viewstate map). That might lead to @PreDestroy >> > getting >> > invoked multiple times. >> >>> >> >>> >> >>> Now another case: a user opens a page.xhtml in his browser. A >> > @ViewScoped bean gets called and if ClientSideStateSaving is activated >> > it gets >> > serialized and stored inside the html page. The user just goes away or >> > closes >> > his browser and this bean will never get the @PreDestroy being called. >> >>> >> >>> Something similar happens with all @SessionScoped or session related >> > beans if a server shuts down and passivates it's sessions. If there is a >> > redeployment inbetween which is not binary compatible, then those beans >> > will >> > never get resurrected and never get properly destroyed because of that. >> >>> >> >>> It get's even worse if you think about Session replication to >> > multiple nodes. On which node should the @PreDestroy being called? And >> > at what >> > time? >> >>> >> >>> >> >>> Again: not sure if you did hit any of the 'natural' reasons or >> > if it is really a bug. If it's a bug then not in CODI but the JSF impls. >> >>> >> >>> >> >>> >> >>> LieGrue, >> >>> strub >> >>> >> >>>> ________________________________ >> >>>> From: Radu Creanga <[email protected]> >> >>>> To: [email protected] >> >>>> Sent: Friday, November 16, 2012 3:25 PM >> >>>> Subject: CODI: @ViewScoped implementation possibly flawed. >> >>>> >> >>>> >> >>>> Dear CODI devs, >> >>>> >> >>>> >> >>>> Please allow me to bring to your attention a possible flaw in the >> > implementation of @ViewScoped. I apologize in advance if this is not the >> > case. >> >>>> >> >>>> >> >>>> The class >> > org.apache.myfaces.extensions.cdi.jsf2.impl.scope.view.ViewScopedContext >> > provides the life-cycle for @ViewScoped annotated objects. An >> > implementation of >> > Context, according to the documentation: >> >>>> >> >>>> >> >>>> "[...] the context object is responsible for destroying any >> > contextual instance it creates by passing the instance to >> > Contextual.destroy(Object, CreationalContext). A destroyed instance must >> > not >> > subsequently be returned by get(). The context object must pass the same >> > instance of CreationalContext to Contextual.destroy() that it passed to >> > Contextual.create() when it created the instance." >> >>>> >> >>>> >> >>>> The ViewScopedContext implementation in question attempts to >> > achieve this requirement by: >> >>>> 1. implementing javax.faces.event.SystemEventListener; >> >>>> 2. registering itself for >> > javax.faces.event.PreDestroyViewMapEvent; >> >>>> 3. destroying Contextual instances upon receiving those events. >> >>>> >> >>>> >> >>>> The problem is that PreDestroyViewMapEvent don't seem to be >> > fired. Intuitively, these events should be fired every time UIViewRoot >> > objects >> > are destroyed, but after spending hours trying to get at least one of >> > them to >> > fire I have seen none, neither on MyFaces, nor on Mojarra JSF >> > implementations. I >> > expected to see these events fired at least on session termination or >> > web-app >> > shut-down, but no luck. >> >>>> >> >>>> >> >>>> The documentation for javax.faces.event.PreDestroyViewMapEvent >> > states: >> >>>> >> >>>> >> >>>> "This event must be published by a call to >> > Application.publishEvent(javax.faces.context.FacesContext, >> > java.lang.Class, >> > java.lang.Object) when the clear method is called on the map returned >> > from >> > UIViewRoot#getViewMap." >> >>>> >> >>>> >> >>>> Is clear()required to be called anywhere? The only reference in the >> > documentation where I could find such a requirement is at >> > FacesContext.setViewRoot(): >> >>>> >> >>>> >> >>>> "If the current UIViewRoot is non-null, and calling equals() >> > on the argument root, passing the current UIViewRoot returns false, the >> > clear >> > method must be called on the Map returned from UIViewRoot#getViewMap." >> >>>> >> >>>> >> >>>> If this is the only place where clear()on the view map is called (I >> > suspect it is since I could not get any of these events to fire), which >> > is only >> > during the Invoke Application phase of the request processing lifecycle >> > and >> > during the Restore View phase of the request processing lifecycle >> > (especially >> > when a new root component is created), then most if not all Contextual >> > instances >> > created by >> > >> > org.apache.myfaces.extensions.cdi.jsf2.impl.scope.view.ViewScopedContextare >> > not >> > disposed properly. >> >>>> >> >>>> >> >>>> I was lead to this while trying to implement my own @ViewScoped >> > context and could not verify that the instances I create are ever >> > destroyed. >> > This lead me to post a question on stackoverflow.com, from where I was >> > suggested >> > I take a look at the CODI implementation. From what I can tell, both >> > implementations do not properly dispose of contextual objects they >> > create, while >> > offering the end-user behaviour they advertise. >> >>>> >> >>>> >> >>>> Hopefully, this is useful insight and I would be honoured to be >> > permitted to get involved in coming up with a solution, assuming a >> > problem >> > exists. >> >>>> >> >>>> Radu Creanga >> >>>> >> >>>> >> >>>> >> >> >> > > >
