myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ssilv...@redhat.com
Subject Re: CODI: @ViewScoped implementation possibly flawed.
Date Fri, 16 Nov 2012 17:14:42 GMT
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 <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