glassfish
  1. glassfish
  2. GLASSFISH-15809

JSF PhaseListener executed for each virtual host

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: v3.0.1
    • Fix Version/s: 3.1.1_b01
    • Component/s: jsf
    • Labels:
      None
    • Environment:

      We're running GlassFish 3.0.1 with JSF 2.1.0b11 on Windows 2007/7 64-bit.

      Description

      We recently spend 2 months changing from JSF 1.2 to 2.0 and GlassFish 2 to 3.0.1. When we were finish with the conversion and ready to go live, we realised that JSF and virtual servers doesn't work together at all on GlassFish 3.0.1 with JSF 2.0. Luckily, this happened on January 27, 2011, the same day that JSF 2.1.0b11 became available with this fix included: http://java.net/jira/browse/GLASSFISH-11984. Otherwise we would have been completely screwed.

      However, it didn't take many hours online, before we realised that something was horrible wrong. We have 6 virtual servers in addition to the default. We can't run with less than that. The problem is that all of our PhaseListeners are now executed one time per virtual server for a total of 7 times per request. And not just ours. PhaseListeners included in other projects, such as PrettyFaces, as well.

      The problem has forced us to roll back to GF 2 and our 2 months old code once again.

      1. 20110217-1558-i_gf_15809.patch
        31 kB
        Ed Burns
      2. 20110222-1610-i_gf_15809.patch
        23 kB
        Ed Burns
      3. i_gf_15809-workaround.txt
        12 kB
        Ed Burns

        Issue Links

          Activity

          Hide
          Ed Burns added a comment -

          The lack of a dev test bites us again.

          Show
          Ed Burns added a comment - The lack of a dev test bites us again.
          Hide
          Ed Burns added a comment -

          Checking out the 2.1.0 branch now.

          svn checkout https://svn.java.net/svn/mojarra~svn/branches/MOJARRA_2_1X_ROLLING mojarra-MOJARRA_2_1X_ROLLING

          Sheetal, make sure you do the same.

          Show
          Ed Burns added a comment - Checking out the 2.1.0 branch now. svn checkout https://svn.java.net/svn/mojarra~svn/branches/MOJARRA_2_1X_ROLLING mojarra-MOJARRA_2_1X_ROLLING Sheetal, make sure you do the same.
          Hide
          Ed Burns added a comment -

          Thankfully, Sheetal's patch for JAVASERVERFACES-1907 looks pretty good. I'm making a couple of small changes to ease the process of iteratively developing a reproducer for this issue (15809) and I'll attach that as a patch to 1907.

          Show
          Ed Burns added a comment - Thankfully, Sheetal's patch for JAVASERVERFACES-1907 looks pretty good. I'm making a couple of small changes to ease the process of iteratively developing a reproducer for this issue (15809) and I'll attach that as a patch to 1907.
          Hide
          Ed Burns added a comment -

          I'm going to attempt a workaround along the following lines. Provide a lifecyclefactory that decorates the original one and ensures there is one instance of each kind of lifecycle type per ServletContext.

          Show
          Ed Burns added a comment - I'm going to attempt a workaround along the following lines. Provide a lifecyclefactory that decorates the original one and ensures there is one instance of each kind of lifecycle type per ServletContext.
          Hide
          Ed Burns added a comment -

          Still getting the kinks worked out of JAVASERVERFACES-1907, working around little league practices.

          Show
          Ed Burns added a comment - Still getting the kinks worked out of JAVASERVERFACES-1907 , working around little league practices.
          Hide
          Ed Burns added a comment -

          After fixing JAVASERVERFACES-1907, I have a solid, automatable, reproducer for this issue.

          container.name=glassfishV3.1_no_cluster

          ant container.start
          ant -Dinstance.numbers=1,2,3,4,5,6 create.virtual.servers
          cd jsf-ri/systest-per-webapp
          ant -Dcreate-virtual-servers=false -Dinstance.numbers=1,2,3,4,5,6 -Dapplication=replace-lifecycle install-virtual-server
          ant -Dcreate-virtual-servers=false -Dinstance.numbers=1,2,3,4,5,6 -Dapplication=replace-lifecycle test-virtual-server
          This shows the phaseListener getting called 6 times.

          The workaround, which I will produce now, is to provide a custom LifecycleFactory instance that correctly handles the virtual server case.

          Show
          Ed Burns added a comment - After fixing JAVASERVERFACES-1907 , I have a solid, automatable, reproducer for this issue. container.name=glassfishV3.1_no_cluster ant container.start ant -Dinstance.numbers=1,2,3,4,5,6 create.virtual.servers cd jsf-ri/systest-per-webapp ant -Dcreate-virtual-servers=false -Dinstance.numbers=1,2,3,4,5,6 -Dapplication=replace-lifecycle install-virtual-server ant -Dcreate-virtual-servers=false -Dinstance.numbers=1,2,3,4,5,6 -Dapplication=replace-lifecycle test-virtual-server This shows the phaseListener getting called 6 times. The workaround, which I will produce now, is to provide a custom LifecycleFactory instance that correctly handles the virtual server case.
          Hide
          Ed Burns added a comment -

          I will write up the workaround and post it in the bug report.

          Show
          Ed Burns added a comment - I will write up the workaround and post it in the bug report.
          Hide
          Ed Burns added a comment -

          Workaround description in plain text.

          Show
          Ed Burns added a comment - Workaround description in plain text.
          Hide
          Ed Burns added a comment -

          Snapshot, in progress.

          Show
          Ed Burns added a comment - Snapshot, in progress.
          Hide
          Ed Burns added a comment -

          snapshot. ClusterTest fails:

          Running com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase
          Testsuite: com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase
          Tests run: 1, Failures: 1, Errors: 0, Time elapsed: 3.38 sec
          Tests run: 1, Failures: 1, Errors: 0, Time elapsed: 3.38 sec

          Testcase: testSimpleObject took 3.103 sec
          FAILED
          null
          junit.framework.AssertionFailedError
          at com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase.assertSimpleObjectOutput(ClusterNoAgressiveSessionDirtyingTestCase.java:115)
          at com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase.testSimpleObject(ClusterNoAgressiveSessionDirtyingTestCase.java:84)
          at com.sun.faces.htmlunit.AbstractTestCase.runTest(AbstractTestCase.java:628)

          About to stop container.

          Show
          Ed Burns added a comment - snapshot. ClusterTest fails: Running com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase Testsuite: com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase Tests run: 1, Failures: 1, Errors: 0, Time elapsed: 3.38 sec Tests run: 1, Failures: 1, Errors: 0, Time elapsed: 3.38 sec Testcase: testSimpleObject took 3.103 sec FAILED null junit.framework.AssertionFailedError at com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase.assertSimpleObjectOutput(ClusterNoAgressiveSessionDirtyingTestCase.java:115) at com.sun.faces.systest.ClusterNoAgressiveSessionDirtyingTestCase.testSimpleObject(ClusterNoAgressiveSessionDirtyingTestCase.java:84) at com.sun.faces.htmlunit.AbstractTestCase.runTest(AbstractTestCase.java:628) About to stop container.
          Hide
          Ed Burns added a comment -

          Small but fundamental change to FactoryFinder http://java.net/jira/browse/GLASSFISH-15809

          The root cause of our virtual server woes is an invalid assumption
          FactoryFinder has made since its first commit. The assumption. There
          is a 1:1 mapping between ServletContext and web-app ClassLoader. This
          assumption is not valid in the case of virtual servers. In a deployment
          with N virtual servers, there are N ServletContexts for each web-app
          ClassLoader. According to Shing-wai, this is not a bug, and I agree
          with him.

          SECTION: Modified Files
          ----------------------------
          M jsf-api/src/main/java/javax/faces/FactoryFinder.java

          • Instead of having the FactoryManagerCache use ClassLoader alone as the
            key for the FactoryManager instances, use a new class,
            FactoryManagerCacheKey that combines the ClassLoader with the
            ServletContext instance. I had to be careful to handle the case when
            FacesContext.getCurrentInstance() returns null.

          M jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java

          • When an app is undeployed, release its factories, which will cause
            them to be removed from the FactoryManagerCache.

          M jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.javaM jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java
          M jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java
          M jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java
          M jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java
          M jsf-ri/build.xml

          • add deploy target

          M jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java
          M jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java

          • test changes to verify 15809 is fixed.

          M lib/jsf-extensions-test-time.jar

          • Fix a bug in MockFacesContext that doesn't actually release the threadlocal.

          SECTION: Diffs
          ----------------------------
          Index: jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java
          ===================================================================
          — jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java (revision 8870)
          +++ jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java (working copy)
          @@ -117,16 +117,16 @@
          request.setAttribute("reqScopeName", "reqScopeValue");
          response = new MockHttpServletResponse();

          + externalContext =
          + new MockExternalContext(servletContext, request, response);
          + lifecycle = new MockLifecycle();
          + facesContext = new MockFacesContext(externalContext, lifecycle);
          // Set up Faces API Objects
          FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY,
          "com.sun.faces.mock.MockApplicationFactory");
          FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY,
          "com.sun.faces.mock.MockRenderKitFactory");

          • externalContext =
          • new MockExternalContext(servletContext, request, response);
          • lifecycle = new MockLifecycle();
          • facesContext = new MockFacesContext(externalContext, lifecycle);
            ApplicationFactory applicationFactory = (ApplicationFactory)
            FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
            application = (MockApplication) applicationFactory.getApplication();
            Index: jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java
            ===================================================================
              • jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java (revision 8870)
                +++ jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java (working copy)
                @@ -164,18 +164,18 @@
                request.setAttribute("reqScopeName", "reqScopeValue");
                response = new MockHttpServletResponse();

          + externalContext =
          + new MockExternalContext(servletContext, request, response);
          + Map map = new HashMap();
          + externalContext.setRequestParameterMap(map);
          + lifecycle = new MockLifecycle();
          + facesContext = new MockFacesContext(externalContext, lifecycle);
          // Set up Faces API Objects
          FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY,
          "com.sun.faces.mock.MockApplicationFactory");
          FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY,
          "com.sun.faces.mock.MockRenderKitFactory");

          • externalContext =
          • new MockExternalContext(servletContext, request, response);
          • Map map = new HashMap();
          • externalContext.setRequestParameterMap(map);
          • lifecycle = new MockLifecycle();
          • facesContext = new MockFacesContext(externalContext, lifecycle);
            ApplicationFactory applicationFactory = (ApplicationFactory)
            FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
            application = (MockApplication) applicationFactory.getApplication();
            Index: jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java
            ===================================================================
              • jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java (revision 8870)
                +++ jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java (working copy)
                @@ -124,16 +124,16 @@
                request.setAttribute("reqScopeName", "reqScopeValue");
                response = new MockHttpServletResponse();

          + externalContext =
          + new MockExternalContext(servletContext, request, response);
          + lifecycle = new MockLifecycle();
          + facesContext = new MockFacesContext(externalContext, lifecycle);
          // Set up Faces API Objects
          FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY,
          "com.sun.faces.mock.MockApplicationFactory");
          FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY,
          "com.sun.faces.mock.MockRenderKitFactory");

          • externalContext =
          • new MockExternalContext(servletContext, request, response);
          • lifecycle = new MockLifecycle();
          • facesContext = new MockFacesContext(externalContext, lifecycle);
            ApplicationFactory applicationFactory = (ApplicationFactory)
            FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
            application = (MockApplication) applicationFactory.getApplication();
            Index: jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java
            ===================================================================
              • jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java (revision 8870)
                +++ jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java (working copy)
                @@ -142,16 +142,16 @@
                pageContext.initialize(servlet, request, response, null,
                true, 1024, true);

          + externalContext =
          + new MockExternalContext(servletContext, request, response);
          + externalContext.setRequestParameterMap(new HashMap());
          + lifecycle = new MockLifecycle();
          + facesContext = new MockFacesContext(externalContext, lifecycle);
          // Set up Faces API Objects
          FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY,
          "com.sun.faces.mock.MockApplicationFactory");
          FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY,
          "com.sun.faces.mock.MockRenderKitFactory");

          • externalContext =
          • new MockExternalContext(servletContext, request, response);
          • externalContext.setRequestParameterMap(new HashMap());
          • lifecycle = new MockLifecycle();
          • facesContext = new MockFacesContext(externalContext, lifecycle);
            ApplicationFactory applicationFactory = (ApplicationFactory)
            FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
            application = (MockApplication) applicationFactory.getApplication();
            Index: jsf-api/src/main/java/javax/faces/FactoryFinder.java
            ===================================================================
              • jsf-api/src/main/java/javax/faces/FactoryFinder.java (revision 8870)
                +++ jsf-api/src/main/java/javax/faces/FactoryFinder.java (working copy)
                @@ -67,6 +67,8 @@
                import java.lang.reflect.Constructor;
                import java.net.URL;
                import java.net.URLConnection;
                +import java.util.Set;
                +import javax.faces.context.FacesContext;

          /**
          @@ -680,8 +682,8 @@
          */
          private static final class FactoryManagerCache {

          • private ConcurrentMap<ClassLoader,Future<FactoryManager>> applicationMap =
          • new ConcurrentHashMap<ClassLoader, Future<FactoryManager>>();
            + private ConcurrentMap<FactoryManagerCacheKey,Future<FactoryManager>> applicationMap =
            + new ConcurrentHashMap<FactoryManagerCacheKey, Future<FactoryManager>>();

          // ------------------------------------------------------ Public Methods
          @@ -689,8 +691,10 @@

          private FactoryManager getApplicationFactoryManager(ClassLoader cl) {

          + FactoryManagerCacheKey key = new FactoryManagerCacheKey(cl, applicationMap);
          +
          while (true) {

          • Future<FactoryManager> factories = applicationMap.get(cl);
            + Future<FactoryManager> factories = applicationMap.get(key);
            if (factories == null) {
            Callable<FactoryManager> callable =
            new Callable<FactoryManager>() {
            @@ -702,7 +706,7 @@

          FutureTask<FactoryManager> ft =
          new FutureTask<FactoryManager>(callable);

          • factories = applicationMap.putIfAbsent(cl, ft);
            + factories = applicationMap.putIfAbsent(key, ft);
            if (factories == null) { factories = ft; ft.run(); @@ -717,14 +721,14 @@ ce.toString(), ce); }
          • applicationMap.remove(cl);
            + applicationMap.remove(key);
            } catch (InterruptedException ie) {
            if (LOGGER.isLoggable(Level.FINEST)) { LOGGER.log(Level.FINEST, ie.toString(), ie); }
          • applicationMap.remove(cl);
            + applicationMap.remove(key);
            } catch (ExecutionException ee) { throw new FacesException(ee); }

            @@ -735,14 +739,89 @@

          public void removeApplicationFactoryManager(ClassLoader cl)

          { + FactoryManagerCacheKey key = new FactoryManagerCacheKey(cl, applicationMap); - applicationMap.remove(cl); + applicationMap.remove(key); }

          } // END FactoryCache

          + private static final class FactoryManagerCacheKey {
          + private ClassLoader cl;
          + private Object context;

          + private static final String KEY = FactoryFinder.class.getName() + "." ++ FactoryManagerCacheKey.class.getSimpleName();
          +
          + public FactoryManagerCacheKey(ClassLoader cl,
          + Map<FactoryManagerCacheKey,Future<FactoryManager>> factoryMap) {
          + this.cl = cl;
          + FacesContext facesContext = FacesContext.getCurrentInstance();
          + if (null != facesContext) {
          + Map<String, Object> appMap = facesContext.getExternalContext().getApplicationMap();
          + Object val = appMap.get(KEY);
          + if (null == val)

          { + context = new Long(System.currentTimeMillis()); + appMap.put(KEY, context); + }

          else

          { + context = val; + }

          + } else {
          + // We don't have a FacesContext.
          + // Our only recourse is to inspect the keys of the
          + // factoryMap and see if any of them has a classloader
          + // equal to our argument cl.
          + Set<FactoryManagerCacheKey> keys = factoryMap.keySet();
          + FactoryManagerCacheKey match = null;
          + for (FactoryManagerCacheKey cur : keys) {
          + if (this.cl.equals(cur.cl)) {
          + match = cur;
          + if (null != match)

          { + LOGGER.log(Level.WARNING, "Multiple JSF Applications found on same ClassLoader. Unable to safely determine which FactoryManager instance to use. Defaulting to first match."); + }

          + }
          + }
          + if (null != match)

          { + this.context = match.context; + }

          + }
          + }
          +
          + private FactoryManagerCacheKey() {}
          +
          + @Override
          + public boolean equals(Object obj) {
          + if (obj == null)

          { + return false; + }
          + if (getClass() != obj.getClass()) { + return false; + }

          + final FactoryManagerCacheKey other = (FactoryManagerCacheKey) obj;
          + if (this.cl != other.cl && (this.cl == null || !this.cl.equals(other.cl)))

          { + return false; + }
          + if (this.context != other.context && (this.context == null || !this.context.equals(other.context))) { + return false; + }

          + return true;
          + }
          +
          + @Override
          + public int hashCode()

          { + int hash = 7; + hash = 97 * hash + (this.cl != null ? this.cl.hashCode() : 0); + hash = 97 * hash + (this.context != null ? this.context.hashCode() : 0); + return hash; + }

          +
          +
          +
          +
          + }
          +
          +
          /**

          • Maintains the factories for a single web application.
            */
            Index: jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java
            ===================================================================
              • jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java (revision 8870)
                +++ jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java (working copy)
                @@ -235,7 +235,9 @@

          // invoke the FactoryConfigProcessor
          FactoryConfigProcessor fcp = new FactoryConfigProcessor(false);
          + InitFacesContext initContext = new InitFacesContext(sc);
          fcp.process(sc, new DocumentInfo[]

          {new DocumentInfo(d, null)});
          + initContext.release();

          // now get an FacesContext instance from the Factory and ensure
          // no injection occured.
          @@ -264,7 +266,9 @@
          // process the document. This should cause the the InjectionFacesContextFactory
          // to be put into play since there is more than one FacesContextFactory // being configured
          + initContext = new InitFacesContext(sc);
          fcp.process(sc, new DocumentInfo[]{new DocumentInfo(d, null)}

          );
          + initContext.release();

          // get the FacesContextFactory instance. The top-level factory should
          // be the InjectionFacesContextFactory.
          Index: jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java
          ===================================================================
          — jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java (revision 8870)
          +++ jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java (working copy)
          @@ -348,6 +348,7 @@
          ApplicationAssociate.setCurrentInstance(null);
          // Release the initialization mark on this web application
          ConfigManager.getInstance().destory(context);
          + FactoryFinder.releaseFactories();
          if (initContext != null)

          { initContext.release(); }

          Index: jsf-ri/build.xml
          ===================================================================
          — jsf-ri/build.xml (revision 8870)
          +++ jsf-ri/build.xml (working copy)
          @@ -700,7 +700,7 @@

          <target name="passthru">

          • <ant antfile="build-tests.xml" target="execute.cactus.tests"
            + <ant antfile="build-tests.xml" target="passthru"
            inheritAll="true">
            <property name="force.no.cluster" value="true" />
            </ant>
            @@ -715,6 +715,14 @@

          </target>

          + <target name="deploy">
          + <ant antfile="build-tests.xml" target="deploy"
          + inheritAll="true">
          + <property name="force.no.cluster" value="true" />
          + </ant>
          +
          + </target>
          +
          <target name="prepare.cactus.webapp">
          <ant antfile="build-tests.xml" target="prepare.test.webapp"/>
          </target>
          Index: jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java
          ===================================================================
          — jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java (revision 8870)
          +++ jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java (working copy)
          @@ -43,6 +43,7 @@
          import javax.faces.event.PhaseEvent;
          import javax.faces.event.PhaseId;
          import javax.faces.event.PhaseListener;
          +import java.util.Map;

          public class SimplePhaseListener implements PhaseListener {

          @@ -57,8 +58,12 @@

          public void beforePhase(PhaseEvent event)

          { - event.getFacesContext().getExternalContext().getRequestMap().put("beforePhase", - "beforePhase"); + Map<String, Object> requestMap = + event.getFacesContext().getExternalContext().getRequestMap(); + String message = requestMap.containsKey("beforePhase") ? + requestMap.get("beforePhase").toString() : ""; + requestMap.put("beforePhase", + message + " beforePhase"); event.getFacesContext().getExternalContext().getRequestMap().put("lifecycleImpl", event.getSource()); }

          Index: jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java
          ===================================================================
          — jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java (revision 8870)
          +++ jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java (working copy)
          @@ -128,7 +128,10 @@

          public void testReplaceLifecycle() throws Exception

          { HtmlPage page = getPage("/faces/test.jsp"); - assertTrue(-1 != page.asText().indexOf("beforePhase")); + String pageText = page.asText(); + assertTrue(-1 != pageText.indexOf("beforePhase")); + // Ensure the phaseListener is only called once. + assertTrue(!pageText.matches("(?s).*beforePhase.*beforePhase.*")); }

          Index: lib/jsf-extensions-test-time.jar
          ===================================================================
          Cannot display: file marked as a binary type.
          svn:mime-type = application/octet-stream

          Show
          Ed Burns added a comment - Small but fundamental change to FactoryFinder http://java.net/jira/browse/GLASSFISH-15809 The root cause of our virtual server woes is an invalid assumption FactoryFinder has made since its first commit. The assumption. There is a 1:1 mapping between ServletContext and web-app ClassLoader. This assumption is not valid in the case of virtual servers. In a deployment with N virtual servers, there are N ServletContexts for each web-app ClassLoader. According to Shing-wai, this is not a bug, and I agree with him. SECTION: Modified Files ---------------------------- M jsf-api/src/main/java/javax/faces/FactoryFinder.java Instead of having the FactoryManagerCache use ClassLoader alone as the key for the FactoryManager instances, use a new class, FactoryManagerCacheKey that combines the ClassLoader with the ServletContext instance. I had to be careful to handle the case when FacesContext.getCurrentInstance() returns null. M jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java When an app is undeployed, release its factories, which will cause them to be removed from the FactoryManagerCache. M jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.javaM jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java M jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java M jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java M jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java M jsf-ri/build.xml add deploy target M jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java M jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java test changes to verify 15809 is fixed. M lib/jsf-extensions-test-time.jar Fix a bug in MockFacesContext that doesn't actually release the threadlocal. SECTION: Diffs ---------------------------- Index: jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java =================================================================== — jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java (revision 8870) +++ jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java (working copy) @@ -117,16 +117,16 @@ request.setAttribute("reqScopeName", "reqScopeValue"); response = new MockHttpServletResponse(); + externalContext = + new MockExternalContext(servletContext, request, response); + lifecycle = new MockLifecycle(); + facesContext = new MockFacesContext(externalContext, lifecycle); // Set up Faces API Objects FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY, "com.sun.faces.mock.MockApplicationFactory"); FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY, "com.sun.faces.mock.MockRenderKitFactory"); externalContext = new MockExternalContext(servletContext, request, response); lifecycle = new MockLifecycle(); facesContext = new MockFacesContext(externalContext, lifecycle); ApplicationFactory applicationFactory = (ApplicationFactory) FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY); application = (MockApplication) applicationFactory.getApplication(); Index: jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java =================================================================== jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java (revision 8870) +++ jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java (working copy) @@ -164,18 +164,18 @@ request.setAttribute("reqScopeName", "reqScopeValue"); response = new MockHttpServletResponse(); + externalContext = + new MockExternalContext(servletContext, request, response); + Map map = new HashMap(); + externalContext.setRequestParameterMap(map); + lifecycle = new MockLifecycle(); + facesContext = new MockFacesContext(externalContext, lifecycle); // Set up Faces API Objects FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY, "com.sun.faces.mock.MockApplicationFactory"); FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY, "com.sun.faces.mock.MockRenderKitFactory"); externalContext = new MockExternalContext(servletContext, request, response); Map map = new HashMap(); externalContext.setRequestParameterMap(map); lifecycle = new MockLifecycle(); facesContext = new MockFacesContext(externalContext, lifecycle); ApplicationFactory applicationFactory = (ApplicationFactory) FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY); application = (MockApplication) applicationFactory.getApplication(); Index: jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java =================================================================== jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java (revision 8870) +++ jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java (working copy) @@ -124,16 +124,16 @@ request.setAttribute("reqScopeName", "reqScopeValue"); response = new MockHttpServletResponse(); + externalContext = + new MockExternalContext(servletContext, request, response); + lifecycle = new MockLifecycle(); + facesContext = new MockFacesContext(externalContext, lifecycle); // Set up Faces API Objects FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY, "com.sun.faces.mock.MockApplicationFactory"); FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY, "com.sun.faces.mock.MockRenderKitFactory"); externalContext = new MockExternalContext(servletContext, request, response); lifecycle = new MockLifecycle(); facesContext = new MockFacesContext(externalContext, lifecycle); ApplicationFactory applicationFactory = (ApplicationFactory) FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY); application = (MockApplication) applicationFactory.getApplication(); Index: jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java =================================================================== jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java (revision 8870) +++ jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java (working copy) @@ -142,16 +142,16 @@ pageContext.initialize(servlet, request, response, null, true, 1024, true); + externalContext = + new MockExternalContext(servletContext, request, response); + externalContext.setRequestParameterMap(new HashMap()); + lifecycle = new MockLifecycle(); + facesContext = new MockFacesContext(externalContext, lifecycle); // Set up Faces API Objects FactoryFinder.setFactory(FactoryFinder.APPLICATION_FACTORY, "com.sun.faces.mock.MockApplicationFactory"); FactoryFinder.setFactory(FactoryFinder.RENDER_KIT_FACTORY, "com.sun.faces.mock.MockRenderKitFactory"); externalContext = new MockExternalContext(servletContext, request, response); externalContext.setRequestParameterMap(new HashMap()); lifecycle = new MockLifecycle(); facesContext = new MockFacesContext(externalContext, lifecycle); ApplicationFactory applicationFactory = (ApplicationFactory) FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY); application = (MockApplication) applicationFactory.getApplication(); Index: jsf-api/src/main/java/javax/faces/FactoryFinder.java =================================================================== jsf-api/src/main/java/javax/faces/FactoryFinder.java (revision 8870) +++ jsf-api/src/main/java/javax/faces/FactoryFinder.java (working copy) @@ -67,6 +67,8 @@ import java.lang.reflect.Constructor; import java.net.URL; import java.net.URLConnection; +import java.util.Set; +import javax.faces.context.FacesContext; /** @@ -680,8 +682,8 @@ */ private static final class FactoryManagerCache { private ConcurrentMap<ClassLoader,Future<FactoryManager>> applicationMap = new ConcurrentHashMap<ClassLoader, Future<FactoryManager>>(); + private ConcurrentMap<FactoryManagerCacheKey,Future<FactoryManager>> applicationMap = + new ConcurrentHashMap<FactoryManagerCacheKey, Future<FactoryManager>>(); // ------------------------------------------------------ Public Methods @@ -689,8 +691,10 @@ private FactoryManager getApplicationFactoryManager(ClassLoader cl) { + FactoryManagerCacheKey key = new FactoryManagerCacheKey(cl, applicationMap); + while (true) { Future<FactoryManager> factories = applicationMap.get(cl); + Future<FactoryManager> factories = applicationMap.get(key); if (factories == null) { Callable<FactoryManager> callable = new Callable<FactoryManager>() { @@ -702,7 +706,7 @@ FutureTask<FactoryManager> ft = new FutureTask<FactoryManager>(callable); factories = applicationMap.putIfAbsent(cl, ft); + factories = applicationMap.putIfAbsent(key, ft); if (factories == null) { factories = ft; ft.run(); @@ -717,14 +721,14 @@ ce.toString(), ce); } applicationMap.remove(cl); + applicationMap.remove(key); } catch (InterruptedException ie) { if (LOGGER.isLoggable(Level.FINEST)) { LOGGER.log(Level.FINEST, ie.toString(), ie); } applicationMap.remove(cl); + applicationMap.remove(key); } catch (ExecutionException ee) { throw new FacesException(ee); } @@ -735,14 +739,89 @@ public void removeApplicationFactoryManager(ClassLoader cl) { + FactoryManagerCacheKey key = new FactoryManagerCacheKey(cl, applicationMap); - applicationMap.remove(cl); + applicationMap.remove(key); } } // END FactoryCache + private static final class FactoryManagerCacheKey { + private ClassLoader cl; + private Object context; + private static final String KEY = FactoryFinder.class.getName() + "." ++ FactoryManagerCacheKey.class.getSimpleName(); + + public FactoryManagerCacheKey(ClassLoader cl, + Map<FactoryManagerCacheKey,Future<FactoryManager>> factoryMap) { + this.cl = cl; + FacesContext facesContext = FacesContext.getCurrentInstance(); + if (null != facesContext) { + Map<String, Object> appMap = facesContext.getExternalContext().getApplicationMap(); + Object val = appMap.get(KEY); + if (null == val) { + context = new Long(System.currentTimeMillis()); + appMap.put(KEY, context); + } else { + context = val; + } + } else { + // We don't have a FacesContext. + // Our only recourse is to inspect the keys of the + // factoryMap and see if any of them has a classloader + // equal to our argument cl. + Set<FactoryManagerCacheKey> keys = factoryMap.keySet(); + FactoryManagerCacheKey match = null; + for (FactoryManagerCacheKey cur : keys) { + if (this.cl.equals(cur.cl)) { + match = cur; + if (null != match) { + LOGGER.log(Level.WARNING, "Multiple JSF Applications found on same ClassLoader. Unable to safely determine which FactoryManager instance to use. Defaulting to first match."); + } + } + } + if (null != match) { + this.context = match.context; + } + } + } + + private FactoryManagerCacheKey() {} + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + final FactoryManagerCacheKey other = (FactoryManagerCacheKey) obj; + if (this.cl != other.cl && (this.cl == null || !this.cl.equals(other.cl))) { + return false; + } + if (this.context != other.context && (this.context == null || !this.context.equals(other.context))) { + return false; + } + return true; + } + + @Override + public int hashCode() { + int hash = 7; + hash = 97 * hash + (this.cl != null ? this.cl.hashCode() : 0); + hash = 97 * hash + (this.context != null ? this.context.hashCode() : 0); + return hash; + } + + + + + } + + /** Maintains the factories for a single web application. */ Index: jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java =================================================================== jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java (revision 8870) +++ jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java (working copy) @@ -235,7 +235,9 @@ // invoke the FactoryConfigProcessor FactoryConfigProcessor fcp = new FactoryConfigProcessor(false); + InitFacesContext initContext = new InitFacesContext(sc); fcp.process(sc, new DocumentInfo[] {new DocumentInfo(d, null)}); + initContext.release(); // now get an FacesContext instance from the Factory and ensure // no injection occured. @@ -264,7 +266,9 @@ // process the document. This should cause the the InjectionFacesContextFactory // to be put into play since there is more than one FacesContextFactory // being configured + initContext = new InitFacesContext(sc); fcp.process(sc, new DocumentInfo[]{new DocumentInfo(d, null)} ); + initContext.release(); // get the FacesContextFactory instance. The top-level factory should // be the InjectionFacesContextFactory. Index: jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java =================================================================== — jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java (revision 8870) +++ jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java (working copy) @@ -348,6 +348,7 @@ ApplicationAssociate.setCurrentInstance(null); // Release the initialization mark on this web application ConfigManager.getInstance().destory(context); + FactoryFinder.releaseFactories(); if (initContext != null) { initContext.release(); } Index: jsf-ri/build.xml =================================================================== — jsf-ri/build.xml (revision 8870) +++ jsf-ri/build.xml (working copy) @@ -700,7 +700,7 @@ <target name="passthru"> <ant antfile="build-tests.xml" target="execute.cactus.tests" + <ant antfile="build-tests.xml" target="passthru" inheritAll="true"> <property name="force.no.cluster" value="true" /> </ant> @@ -715,6 +715,14 @@ </target> + <target name="deploy"> + <ant antfile="build-tests.xml" target="deploy" + inheritAll="true"> + <property name="force.no.cluster" value="true" /> + </ant> + + </target> + <target name="prepare.cactus.webapp"> <ant antfile="build-tests.xml" target="prepare.test.webapp"/> </target> Index: jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java =================================================================== — jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java (revision 8870) +++ jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java (working copy) @@ -43,6 +43,7 @@ import javax.faces.event.PhaseEvent; import javax.faces.event.PhaseId; import javax.faces.event.PhaseListener; +import java.util.Map; public class SimplePhaseListener implements PhaseListener { @@ -57,8 +58,12 @@ public void beforePhase(PhaseEvent event) { - event.getFacesContext().getExternalContext().getRequestMap().put("beforePhase", - "beforePhase"); + Map<String, Object> requestMap = + event.getFacesContext().getExternalContext().getRequestMap(); + String message = requestMap.containsKey("beforePhase") ? + requestMap.get("beforePhase").toString() : ""; + requestMap.put("beforePhase", + message + " beforePhase"); event.getFacesContext().getExternalContext().getRequestMap().put("lifecycleImpl", event.getSource()); } Index: jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java =================================================================== — jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java (revision 8870) +++ jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java (working copy) @@ -128,7 +128,10 @@ public void testReplaceLifecycle() throws Exception { HtmlPage page = getPage("/faces/test.jsp"); - assertTrue(-1 != page.asText().indexOf("beforePhase")); + String pageText = page.asText(); + assertTrue(-1 != pageText.indexOf("beforePhase")); + // Ensure the phaseListener is only called once. + assertTrue(!pageText.matches("(?s).*beforePhase.*beforePhase.*")); } Index: lib/jsf-extensions-test-time.jar =================================================================== Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream
          Hide
          Jason Lee added a comment -

          The changes look good to me.

          r=jdlee

          Show
          Jason Lee added a comment - The changes look good to me. r=jdlee
          Hide
          Ed Burns added a comment -

          Sending jsf-api/src/main/java/javax/faces/FactoryFinder.java
          Sending jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java
          Sending jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java
          Sending jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java
          Sending jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java
          Sending jsf-ri/build.xml
          Sending jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java
          Sending jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java
          Sending jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java
          Sending jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java
          Sending lib/jsf-extensions-test-time.jar
          Transmitting file data ...........
          Committed revision 8871.

          Show
          Ed Burns added a comment - Sending jsf-api/src/main/java/javax/faces/FactoryFinder.java Sending jsf-api/src/test/java/javax/faces/component/NamingContainerTestCase.java Sending jsf-api/src/test/java/javax/faces/component/UIComponentTestCase.java Sending jsf-api/src/test/java/javax/faces/validator/ValidatorTestCase.java Sending jsf-api/src/test/java/javax/faces/webapp/TagTestCaseBase.java Sending jsf-ri/build.xml Sending jsf-ri/src/main/java/com/sun/faces/config/ConfigureListener.java Sending jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/ReplaceLifecycleTestCase.java Sending jsf-ri/systest-per-webapp/replace-lifecycle/src/java/com/sun/faces/systest/SimplePhaseListener.java Sending jsf-ri/test/com/sun/faces/config/TestFactoryInjection.java Sending lib/jsf-extensions-test-time.jar Transmitting file data ........... Committed revision 8871.
          Hide
          Ed Burns added a comment -

          add jsf_2_1_1 to status whiteboard

          Show
          Ed Burns added a comment - add jsf_2_1_1 to status whiteboard
          Hide
          tedgoddard added a comment -

          The following WARNING is seen when visiting the first page in an application running on Tomcat:

          Mar 22, 2011 4:35:49 PM javax.faces.FactoryFinder$FactoryManagerCacheKey <init>
          WARNING: Multiple JSF Applications found on same ClassLoader.  Unable to safely determine which FactoryManager instance to use. Defaulting to first match.

          It has no functional impact.

          Show
          tedgoddard added a comment - The following WARNING is seen when visiting the first page in an application running on Tomcat: Mar 22, 2011 4:35:49 PM javax.faces.FactoryFinder$FactoryManagerCacheKey <init> WARNING: Multiple JSF Applications found on same ClassLoader.  Unable to safely determine which FactoryManager instance to use. Defaulting to first match. It has no functional impact.
          Hide
          Scott Fordin added a comment -

          Does this still need to be added to the Release Notes? If so, I need more information.

          Show
          Scott Fordin added a comment - Does this still need to be added to the Release Notes? If so, I need more information.
          Hide
          Ed Burns added a comment -

          Integrated into GlassFish 3.1.1

          Show
          Ed Burns added a comment - Integrated into GlassFish 3.1.1
          Hide
          Scott Fordin added a comment -

          Still need a more concise workaround.

          Show
          Scott Fordin added a comment - Still need a more concise workaround.
          Hide
          Scott Fordin added a comment -

          Added issue to 3.1 Known Issues section of 3.1-3.1.1 Release Notes. In the absence of a more concise workaround, I linked to the i_gf_15809-workaround.txt file that is attached to this issue.

          Show
          Scott Fordin added a comment - Added issue to 3.1 Known Issues section of 3.1-3.1.1 Release Notes. In the absence of a more concise workaround, I linked to the i_gf_15809-workaround.txt file that is attached to this issue.
          Hide
          bjornstenfeldt added a comment -

          I'm finding it rather frustrating that I'm now getting around 1000 lines of "Multiple JSF Applications found on same ClassLoader. Unable to safely determine which FactoryManager instance to use. Defaulting to first match." in my log, every single second. Maybe a context-param in web.xml to disable this warning?

          Show
          bjornstenfeldt added a comment - I'm finding it rather frustrating that I'm now getting around 1000 lines of "Multiple JSF Applications found on same ClassLoader. Unable to safely determine which FactoryManager instance to use. Defaulting to first match." in my log, every single second. Maybe a context-param in web.xml to disable this warning?
          Hide
          Ed Burns added a comment -

          Hello Björn,

          I am sincerely sorry for your frustration.

          Is it acceptable to request that you modify your logging configuration, as described in <http://wikis.sun.com/display/GlassFish/JavaServerFacesRI#JavaServerFacesRI-HowcanIturnonlogging%3F> to prevent that logging message from being displayed?

          Specifically, modify your logging.properties like this:

          — logging.properties 2011-07-19 21:42:46.000000000 -0400
          +++ logging_GLASSFISH-15809.properties 2011-09-26 11:40:23.000000000 -0400
          @@ -113,3 +113,4 @@
          javax.enterprise.system.ssl.security.level=INFO
          ShoalLogger.level=CONFIG
          org.eclipse.persistence.session.level=INFO
          +javax.faces.level=INFO

          Does that help?

          Show
          Ed Burns added a comment - Hello Björn, I am sincerely sorry for your frustration. Is it acceptable to request that you modify your logging configuration, as described in < http://wikis.sun.com/display/GlassFish/JavaServerFacesRI#JavaServerFacesRI-HowcanIturnonlogging%3F > to prevent that logging message from being displayed? Specifically, modify your logging.properties like this: — logging.properties 2011-07-19 21:42:46.000000000 -0400 +++ logging_ GLASSFISH-15809 .properties 2011-09-26 11:40:23.000000000 -0400 @@ -113,3 +113,4 @@ javax.enterprise.system.ssl.security.level=INFO ShoalLogger.level=CONFIG org.eclipse.persistence.session.level=INFO +javax.faces.level=INFO Does that help?
          Hide
          bjornstenfeldt added a comment -

          It will definitely fix it (javax.faces.level=SEVERE), but I worry that it might disables a lot of other information or warnings too, which is why I've been hesitating to use this method.

          Show
          bjornstenfeldt added a comment - It will definitely fix it (javax.faces.level=SEVERE), but I worry that it might disables a lot of other information or warnings too, which is why I've been hesitating to use this method.
          Hide
          Ed Burns added a comment -

          I can assure you that even though a logger such as javax.faces seems very "low level", there are very few log messages controlled by that logger. The vast majority of JSF related log messages are in the javax.enterprise.resource.webcontainer.jsf and sub loggers.

          Show
          Ed Burns added a comment - I can assure you that even though a logger such as javax.faces seems very "low level", there are very few log messages controlled by that logger. The vast majority of JSF related log messages are in the javax.enterprise.resource.webcontainer.jsf and sub loggers.
          Hide
          bjornstenfeldt added a comment -

          Alright, I'll give it a chance.

          Show
          bjornstenfeldt added a comment - Alright, I'll give it a chance.

            People

            • Assignee:
              Ed Burns
              Reporter:
              bjornstenfeldt
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: