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 >>>> >>>> >>>> >> >
