You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beehive.apache.org by ri...@apache.org on 2004/09/29 07:44:49 UTC

svn commit: rev 47464 - in incubator/beehive/trunk/netui/src: compiler/org/apache/beehive/netui/compiler pageflow/org/apache/beehive/netui/pageflow

Author: rich
Date: Tue Sep 28 22:44:48 2004
New Revision: 47464

Modified:
   incubator/beehive/trunk/netui/src/compiler/org/apache/beehive/netui/compiler/PageFlowChecker.java
   incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FacesBackingBean.java
   incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FlowController.java
   incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowController.java
   incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowManagedObject.java
   incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/SharedFlowController.java
Log:
- Fixed a concurrency bug, where a page flow's destroy() callback (which calls the user's onDestroy() and does internal uninitialization of things like Control member fields) could overlap the execution of an action method triggered by another request.  The problem was the lack of synchronization on destroy().  It is unsafe to synchronize FlowController.destroy() because it is invoked from a session binding listener; depending on the Servlet container, the session itself may be locked, and this risks a deadlock in the situation where in another request thread our framework synchronizes on the page flow instance and then invokes user code that may (directly or indirectly) synchronize on the session.  Our solution is synchronization on the page flow instance *surrounding* any call that will remove/replace the instance in the session.  It's OK to trigger the session binding listener while the page flow is locked.  Bottom line: we always synchronize on the page flow instance before synchronizing on the session, and never allow it to go the other direction.

- Fixed another concurrency issue that occured when a page flow was requested while another of the same type was being destroyed.  A new instance of the page flow was (correctly) created, but because the previous one had not yet fully uninitialized, the new one was trying to initialize Controls with IDs that overlapped those from the old instance (being destroyed).  The fix for this was simply to ensure that the Control IDs are always instance-specific, instead of being merely specific to the page flow and Control types.

DRT: netui (WinXP)
BB: self (linux)



Modified: incubator/beehive/trunk/netui/src/compiler/org/apache/beehive/netui/compiler/PageFlowChecker.java
==============================================================================
--- incubator/beehive/trunk/netui/src/compiler/org/apache/beehive/netui/compiler/PageFlowChecker.java	(original)
+++ incubator/beehive/trunk/netui/src/compiler/org/apache/beehive/netui/compiler/PageFlowChecker.java	Tue Sep 28 22:44:48 2004
@@ -150,7 +150,11 @@
             {
                 File file = CompilerUtils.getOriginalFile( classDecl );
                 
-                if ( ! jpfFile.equals( file ) )
+                //
+                // Add the dependency if it's a different file and if the file exists (it may have been deleted
+                // sometime after the list of classes in this package got built.
+                //
+                if ( ! jpfFile.equals( file ) && file.exists() )
                 {
                     overlapping.add( file.getName() );
                     overlappingFiles.add( file );

Modified: incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FacesBackingBean.java
==============================================================================
--- incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FacesBackingBean.java	(original)
+++ incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FacesBackingBean.java	Tue Sep 28 22:44:48 2004
@@ -25,6 +25,7 @@
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
 import javax.servlet.ServletContext;
 import java.util.Map;
 import java.util.Collections;
@@ -42,6 +43,12 @@
     {
         HttpServletRequest unwrappedRequest = PageFlowUtils.unwrapMultipart( request );
         ScopedServletUtils.setScopedSessionAttr( InternalConstants.FACES_BACKING_ATTR, this, unwrappedRequest );
+    }
+
+    protected void deleteFromSession( HttpServletRequest request )
+    {
+        HttpServletRequest unwrappedRequest = PageFlowUtils.unwrapMultipart( request );
+        ScopedServletUtils.removeScopedSessionAttr( InternalConstants.FACES_BACKING_ATTR, unwrappedRequest );
     }
 
     public void ensureFailover( HttpServletRequest request )

Modified: incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FlowController.java
==============================================================================
--- incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FlowController.java	(original)
+++ incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/FlowController.java	Tue Sep 28 22:44:48 2004
@@ -1208,6 +1208,11 @@
     {
     }
     
+    protected void delete()
+    {
+        deleteFromSession( getRequest() );
+    }
+    
     /**
      * Used by derived classes to store information on the most recent action executed.
      */ 

Modified: incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowController.java
==============================================================================
--- incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowController.java	(original)
+++ incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowController.java	Tue Sep 28 22:44:48 2004
@@ -20,6 +20,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSessionBindingEvent;
+import javax.servlet.http.HttpSession;
 import javax.servlet.ServletContext;
 import java.lang.reflect.Field;
 
@@ -139,10 +140,8 @@
     /**
      * Remove this instance from the session.
      */ 
-    protected void delete()
+    protected synchronized void deleteFromSession( HttpServletRequest request )
     {
-        HttpServletRequest request = getRequest();
-        
         // This request attribute is used in persistInSession to prevent re-saving of this instance.
         request.setAttribute( DELETING_PAGEFLOW_ATTR, this );
         
@@ -168,7 +167,23 @@
     
     void persistInSession( HttpServletRequest request, HttpServletResponse response, ServletContext servletContext )
     {
-        InternalUtils.setCurrentPageFlow( this, request );
+        PageFlowController currentPageFlow = PageFlowUtils.getCurrentPageFlow( request );
+        
+        if ( currentPageFlow != null && ! currentPageFlow.isOnNestingStack() )
+        {
+            //
+            // We're going to be implicitly destroying the current page flow.  Synchronize on it so we don't mess
+            // with concurrent requests.
+            //
+            synchronized ( currentPageFlow )
+            {
+                InternalUtils.setCurrentPageFlow( this, request );
+            }
+        }
+        else
+        {
+            InternalUtils.setCurrentPageFlow( this, request );
+        }
     }
 
     /**
@@ -630,6 +645,10 @@
         }
     }
     
+    private boolean isOnNestingStack()
+    {
+        return _isOnNestingStack;
+    }
     
     /**
      * Callback when this page flow is removed from the user session.  Causes {@link #onDestroy}

Modified: incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowManagedObject.java
==============================================================================
--- incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowManagedObject.java	(original)
+++ incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/PageFlowManagedObject.java	Tue Sep 28 22:44:48 2004
@@ -43,12 +43,12 @@
 {
     private static final Logger _log = Logger.getInstance( PageFlowManagedObject.class );
     
-    
     /**
      * Reference to the current ServletContext.
      */ 
     private transient ServletContext _servletContext;
 
+    
     /**
      * Initialize transient data that may have been lost during session failover.
      * @exclude public for Portal
@@ -106,11 +106,9 @@
     }
 
     /**
-     * Remove this instance from the session.  The base implementation does not do anything.
+     * Remove this instance from the session.
      */ 
-    protected void delete()
-    {
-    }
+    protected abstract void deleteFromSession( HttpServletRequest request );
     
     /**
      * Stores this object in the user session, in the appropriate place.
@@ -142,6 +140,7 @@
         request = PageFlowUtils.unwrapMultipart( request );
         ControlBeanContext beanContext = JavaControlUtils.getControlBeanContext( request, response, false );
         assert beanContext != null : "ControlBeanContext was not initialized by PageFlowRequestProcessor";
+        String classID = getClass().getName();
         
         for ( Iterator i = controlFields.entrySet().iterator(); i.hasNext(); )
         {
@@ -166,7 +165,7 @@
                     
                     PropertyMap propertyMap = ( PropertyMap ) entry.getValue();
                     Class fieldType = field.getType();
-                    String controlID = getControlID( field, request );
+                    String controlID = getControlID( field, classID );
                     boolean isControlBeanClass = ! fieldType.isInterface();
                     ControlBean bean = JavaControlUtils.createControl( fieldType.getName(), isControlBeanClass,
                                                                        controlID, propertyMap, beanContext );
@@ -184,15 +183,12 @@
     /**
      * Create a unique ID for a given Java Control field.
      */ 
-    private String getControlID( Field controlField, HttpServletRequest request )
+    private String getControlID( Field controlField, String classID )
     {
         StringBuilder controlID = new StringBuilder();
-        ScopedRequest scopedRequest = ScopedServletUtils.unwrapRequest( request );
-        
-        // If this is a ScopedRequest, include the request's scope key in the control ID.
-        if ( scopedRequest != null ) controlID.append( scopedRequest.getScopeKey() ).append( ':' );
-        controlID.append( getClass().getName() ).append( '.' );
-        controlID.append( controlField.getName() );
+        controlID.append( classID );                                  // classname
+        controlID.append( '@' ).append( hashCode() );                 // instance ID
+        controlID.append( '.' ).append( controlField.getName() );     // name of the control field
         return controlID.toString();
     }
     

Modified: incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/SharedFlowController.java
==============================================================================
--- incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/SharedFlowController.java	(original)
+++ incubator/beehive/trunk/netui/src/pageflow/org/apache/beehive/netui/pageflow/SharedFlowController.java	Tue Sep 28 22:44:48 2004
@@ -23,6 +23,7 @@
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
 import javax.servlet.ServletContext;
 
 import org.apache.beehive.netui.pageflow.internal.InternalUtils;
@@ -123,8 +124,8 @@
     /**
      * Remove this instance from the session.
      */ 
-    protected void delete()
+    protected synchronized void deleteFromSession( HttpServletRequest request )
     {
-        PageFlowUtils.deleteSharedFlow( getClass().getName(), getRequest() );
+        PageFlowUtils.deleteSharedFlow( getClass().getName(), request );
     }
 }