[jira] Closed: (MYFACES-750) Need synchronization in LifecycleImpl

["sean schofield (JIRA)" <de...@myfaces.apache.org>]

     [ http://issues.apache.org/jira/browse/MYFACES-750?page=all ]
     
sean schofield closed MYFACES-750:
----------------------------------


> Need synchronization in LifecycleImpl
> -------------------------------------
>
>          Key: MYFACES-750
>          URL: http://issues.apache.org/jira/browse/MYFACES-750
>      Project: MyFaces Core
>         Type: Bug

>   Components: General
>     Versions: 1.1.0
>  Environment: Weblogic 8.1, AIX
>     Reporter: Sam Schneider
>     Assignee: Manfred Geiler
>      Fix For: 1.1.2

>
> Getting this exception when removing a bean as a listener:
> java.util.ConcurrentModificationException
>  at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:462)
>  at java.util.AbstractList$Itr.next(AbstractList.java:433)
>  at java.util.AbstractCollection.remove(AbstractCollection.java:268)
>  at org.apache.myfaces.lifecycle.LifecycleImpl.removePhaseListener(LifecycleImpl.java:411)
>  at com.erac.riskmgmt.ice.web.AbstractPageBean.afterPhase(AbstractPageBean.java:128)
> The method call at AbstractPageBean:128 is getLifecycle().removePhaseListener(this);
>   protected Lifecycle getLifecycle() {
>     String lifecycleId = getExternalContext().getInitParameter("javax.faces.LIFECYCLE_ID");
>     if ((lifecycleId == null) || (lifecycleId.length() == 0)) {
>       lifecycleId = LifecycleFactory.DEFAULT_LIFECYCLE;
>     }
>     LifecycleFactory lifecycleFactory = (LifecycleFactory) FactoryFinder.getFactory(
>         FactoryFinder.LIFECYCLE_FACTORY);
>     return lifecycleFactory.getLifecycle(lifecycleId);
>   }
> Basically what's happening from a request perspective is that we have a custom ViewHandler that creates the backing bean that's registered for a given view id (custom registry using a HashMap that's created as a managed bean and maintained manually in faces-config).  When the backing bean is created it adds itself as a listener and when the AFTER RENDER_RESPONSE is executed the bean removes itself as a listener (as above at line 128 in AbstractPageBean).  No problems are observed in low-load situations, but when we sent the application to benchmarking simulating ~200 "concurrent" users we noticed this problem.  It looks like easiest fix is to synchronize the array list on adding and removing listeners (currently we've worked around this by using an external lock to synchronize add/remove calls from our AbstractPageBean).
> However, It looks like there is also a potential problem with the call to getPhaseListeners() in that it creates a cached array that's later copied back into a *new* ArrayList in addPhaseListener(...) and removePhaseListener(...).  Since there is no synchronization if someone called getPhaseListeners() and then two/N different threads tried to add listeners 1..N listeners could be lost (all would check for a null _phaseListenerList and multiple threads could potentially attempt to create a new _phaseListenerList and add/remove the PhaseListener) -- of course the converse problem would be that two/N different threads tried to remove phase listeners and the 1..N listeners would not be removed (they would be recopied in on another thread).
> What would simplify this whole business greatly would be to remove the cached list and simply synchronize on the ArrayList on the add/remove/getListeners:
>     public void addPhaseListener(PhaseListener phaseListener)
>     {
>         if(phaseListener == null)
>         {
>             throw new NullPointerException("PhaseListener must not be null.");
>         }
>        
> // create _phaseListenerList at class scope -- so no more null-checks for it
>         synchronized(_phaseListenerList) {
>         _phaseListenerList.add(phaseListener);
>         }
>     }
>     public void removePhaseListener(PhaseListener phaseListener)
>     {
>         synchronized(_phaseListenerList) {
>         _phaseListenerList.remove(phaseListener);
>         }
>     }
>     public PhaseListener[] getPhaseListeners()
>     {
>         synchronized(_phaseListenerList) {
>                 return  (PhaseListener[])_phaseListenerList.toArray(new PhaseListener[_phaseListenerList.size()]);
>          }
>     }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira