myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Struberg <strub...@yahoo.de>
Subject Re: CODI: @ViewScoped implementation possibly flawed.
Date Fri, 16 Nov 2012 14:50:08 GMT


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 <rdcrng@gmail.com>
>To: dev@myfaces.apache.org 
>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
>
>
>

Mime
View raw message