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.java
M 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
--
| edward.burns_at_oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/