You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by so...@apache.org on 2013/05/22 06:42:12 UTC

svn commit: r1485062 - /myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java

Author: sobryan
Date: Wed May 22 04:42:12 2013
New Revision: 1485062

URL: http://svn.apache.org/r1485062
Log:
TRINIDAD-2393: GlobalConfiguratorImpl will not always clean up resources

Modified:
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java?rev=1485062&r1=1485061&r2=1485062&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java Wed May 22 04:42:12 2013
@@ -20,8 +20,6 @@ package org.apache.myfaces.trinidadinter
 
 import java.io.IOException;
 
-import java.io.IOException;
-
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
@@ -315,31 +313,14 @@ public final class GlobalConfiguratorImp
     // do per virtual-request stuff
     if (state.get(_REQUEST_TYPE) != null)
     {
-      if (!_isDisabled(ec))
+      try
       {
-        boolean isRedirected = (null != ec.getRequestMap().remove(_REDIRECT_ISSUED));
-        
-        try
-        {
-          //Only end services at the end of a writable response.  This will
-          //generally be RENDER, RESOURCE, and SERVLET.
-          if ((ExternalContextUtils.isResponseWritable(ec) || isRedirected) && !Boolean.TRUE.equals(state.get(_CONFIGURATORS_ABORTED)))
-          {
-            _endConfiguratorServiceRequest(ec);
-          }
-        }
-        finally
-        {
-          //If redirect was issued, we do not want to save the state.  Let it burn..  :D
-          if(!isRedirected)
-          {
-            state.saveState(ec);
-          }
-          
-          _releaseRequestContext(ec);
-        }
+         _endRequest(ec, state);
+      }
+      finally
+      {
+        state.remove(_REQUEST_TYPE);
       }
-      state.remove(_REQUEST_TYPE);
     }
   }
 
@@ -555,18 +536,79 @@ public final class GlobalConfiguratorImp
     }
   }
 
-  private void _releaseRequestContext(ExternalContext ec)
+  private void _endRequest(ExternalContext ec, RequestStateMap state)
   {
-    RequestContext context = RequestContext.getCurrentInstance();
-    if (context != null)
+    if (!_isDisabled(ec))
+    {
+      try
+      {
+        _endConfiguratorServiceRequest(ec, state);
+      }
+      finally
+      {
+        _releaseRequestState(ec);
+      }
+    }
+  }
+
+  private void _endConfiguratorServiceRequest(ExternalContext ec, RequestStateMap state)
+  {
+    boolean isRedirected = (null != ec.getRequestMap().remove(_REDIRECT_ISSUED));
+    
+    try
+    {
+      //Only end services at the end of a writable response.  This will
+      //generally be RENDER, RESOURCE, and SERVLET.
+      
+      //WE had to add a check to see if a redirect was issued in order to handle a bug where endRequest was not
+      //executed on a redirect.
+      if ((ExternalContextUtils.isResponseWritable(ec) || isRedirected) && !Boolean.TRUE.equals(state.get(_CONFIGURATORS_ABORTED)))
+      {
+        _endConfiguratorServices(ec);
+      }
+    }
+    finally
+    {
+      //If redirect was issued, we do not want to save the state.  Let it burn..  :D
+      if(!isRedirected)
+      {
+        state.saveState(ec);
+      }
+    }
+  }
+
+  private void _releaseRequestState(ExternalContext ec)
+  {
+    try
+    {
+      _releaseThreadLocals(ec);
+    }
+    finally
     {
       // ensure that any deferred ComponentReferences are initialized
       _finishComponentReferenceInitialization(ec);
-
-      context.release();
+    }
+  }
+  
+  private void _releaseThreadLocals(ExternalContext ec)
+  {
+    try
+    {
+      _releaseRequestContext(ec);
+    }
+    finally
+    {
       _releaseManagedThreadLocals();
+    }
+  }
+
+  private void _releaseRequestContext(ExternalContext ec)
+  {
+    RequestContext context = RequestContext.getCurrentInstance();
 
-      assert RequestContext.getCurrentInstance() == null;
+    if (context != null)
+    {
+      context.release();
     }
   }
 
@@ -596,17 +638,32 @@ public final class GlobalConfiguratorImp
     
     if ((initializeList != null) && !initializeList.isEmpty())
     {
+      RuntimeException initializationException = null;
+
       for (ComponentReference<?> reference : initializeList)
       {
+        try
+        {
         reference.ensureInitialization();
       }
+        catch (RuntimeException rte)
+        {
+          initializationException = rte;
+        }
+      }
       
       // we've initialized everything, so we're done
       initializeList.clear();
+
+      // rethrow the exception now that we are all done cleaning up
+      if (initializationException != null)
+      {
+        throw initializationException;
+      }
     }
   }
 
-  private void _endConfiguratorServiceRequest(final ExternalContext ec)
+  private void _endConfiguratorServices(final ExternalContext ec)
   {
     // Physical request has now ended
     // Clear the in-request flag