myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: StackOverflowError with MyFaces 2.2
Date Mon, 31 Mar 2014 09:30:17 GMT
Hi Oleg

2014-03-31 10:47 GMT+02:00 Oleg Varaksin <ovaraksin@googlemail.com>:
> Hello Leonardo,
>
> Thanks for your reply. I'm the creator of this component. The component
> belongs to the PrimeFaces Extensions. As I already told you, this was
> working in JSF 2.0, 2.1 and still works in Mojarra 2.2. So, I don't think we
> have a bug there. Do you mean we don't need
>
> @ListenerFor(systemEventClass = PostRestoreStateEvent.class)
>

Yes, because every component is already subscribed to that event. The reason
is the "binding" attribute uses PostRestoreStateEvent to restore the bindings.

In fact, it works in JSF 2.0 and 2.1 because that statement effectively does not
have any effect. In other words, it is just ignored. It fails in
MyFaces 2.2 because
MyFaces takes care of it, which has sense to me.

The implementation done in Mojarra 2.2 has a lot of problems related to
vdl.createComponent(...) as described on:

https://java.net/projects/javaserverfaces-spec-public/lists/users/archive/2014-03/message/16

I think its implementation of vdl.createComponent(...) is incomplete
and insuficient
(this is my personal opinion of course). But other developers has
mentioned before
the need of propagate PostRestoreStateEvent to listeners in the past, so it was
decided to include the code in 2.2.

In JSF 2.2 spec, there are some lines in the list of changes done
between 2.1 and
2.2 that says:

https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1061

"... Clarify that both Application.publishEvent() and the manual
traversal based delivery are required for
publishing the PostRestoreStateEvent. ..."

So, the change is on the spec, but maybe there is a difference between how
MyFaces and Mojarra implemented it (different developers think different).
The problem here is this part is important for performance, so the
implementation was done to avoid unnecessary calls and ensure a fast tree
restore. The code is correct and comply with the spec.

> because the component will receive the PostRestoreStateEvent automatically?
> I'm not sure, but I can check. super.processEvent() ist necessary, I can not
> remove it. Normally, the logic of the parent component is called first and
> then the specific logic of the extended component. That means, if we call
> super.processEvent(), that will call processEvent() in the extended
> component (MasterDetail) again and we have an endless loop -->
> StackOverflowError.
>

I'm 100% sure, because "binding" attribute will not work without that. The
fix that we can do in MyFaces is simple, just check if the listener is the same
component and if that so, do not propagate it. A simple == will not cause any
trouble and will fix the problem in a way that avoid the removal of the code
in your class. But keep in mind we have fallen here in a blurry detail of the
spec, that was clarified in 2.2. A valid fix and the suggested one is also
remove the line from the class, because in fact it has no effect.

regards,

Leonardo Uribe

> Regards.
> Oleg.
>
>
> 2014-03-30 23:44 GMT+02:00 Leonardo Uribe <lu4242@gmail.com>:
>
>> Hi
>>
>> I can see a change that was introduced in JSF 2.2. It has the
>> following description:
>>
>>             // JSF 2.2 vdl.createComponent() requires special handling
>> to refresh
>>             // dynamic parts when refreshing is done. The only way to do
>> it is
>>             // attaching a listener to PostRestoreStateEvent, so we
>> need to do this
>>             // invocation here.
>>             // Do it inside UIComponent.processEvent() is better
>> because in facelets
>>             // UILeaf we can skip this part just overriding the method.
>>
>> In JSF 2.0 and 2.1, component listeners attached to
>> PostRestoreStateEvent do not receive the events, but in JSF 2.2 it was
>> added a code there that propagates PostRestoreStateEvent to the
>> listeners, because this fix is necessary to make
>> vdl.createComponent(...) to work. It seems we have a bug in the
>> component (not a MyFaces Bug), because the same component is
>> subscribed as a listener to PostRestoreStateEvent, which is not
>> necessary, because it always receive the event, it is already
>> subscribed by default.
>>
>> Checking the base class, it shows something like this:
>>
>> @ListenerFor(systemEventClass = PostRestoreStateEvent.class)
>> @ResourceDependency(library = "primefaces-extensions", name =
>> "primefaces-extensions.css")
>> public class MasterDetail extends UIComponentBase {
>>
>> Of course, the @ListenerFor annotation is not necessary. But we could
>> make a simple check to avoid the exception ... Anyway this is
>> something to fix on primefaces, not here, so could you please reply
>> the answer to primefaces forum, so they can fix it?
>>
>> regards,
>>
>> Leonardo Uribe
>>
>>
>> 2014-03-30 23:26 GMT+02:00 Oleg Varaksin <ovaraksin@googlemail.com>:
>> > By the way, it is a common pattern to call in a custom component e.g.
>> > super.processDecodes() in processDecodes() or super.processValidators()
>> > in
>> > processValidators(). This was always working before.
>> >
>> > Am 30.03.2014 23:23, schrieb Oleg Varaksin:
>> >
>> >> Hello MyFaces team,
>> >>
>> >> We get a StackOverflowError since MyFaces 2.x. The stack trace is shown
>> >> here http://forum.primefaces.org/viewtopic.php?f=14&t=36999
>> >>
>> >> The problem: Wenn we call super.processEvent(event) in the
>> >> processEvent()
>> >> of a custom component, we get a recursion which ends in
>> >> StackOverflowError.
>> >>
>> >> @Override
>> >> public void processEvent(ComponentSystemEvent event) throws
>> >> AbortProcessingException {
>> >>     super.processEvent(event);
>> >>     ...
>> >> }
>> >>
>> >> The call super.processEvent(event) is necessary because e.g. Mojarra
>> >> executes there some important code. But if we look at processEvent() in
>> >> the
>> >> MyFaces' UIComponent, it iterates over all listeners for processEvent()
>> >> and
>> >> invokes them. That means, processEvent() of the custom component is
>> >> invoked
>> >> again. Does it work as designed or a is it a bug?
>> >>
>> >> It was working before MyFaces 2.x and it works for all Mojarra
>> >> versions.
>> >>
>> >> Thanks in advance.
>> >> Oleg.
>> >
>> >
>
>

Mime
View raw message