You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by lu...@apache.org on 2008/04/04 00:49:24 UTC
svn commit: r644540 -
/myfaces/core/trunk/api/src/main/java/javax/faces/FactoryFinder.java
Author: lu4242
Date: Thu Apr 3 15:49:22 2008
New Revision: 644540
URL: http://svn.apache.org/viewvc?rev=644540&view=rev
Log:
fix MYFACES-1558 FactoryFinder not thread safe, and has a memory leak
Modified:
myfaces/core/trunk/api/src/main/java/javax/faces/FactoryFinder.java
Modified: myfaces/core/trunk/api/src/main/java/javax/faces/FactoryFinder.java
URL: http://svn.apache.org/viewvc/myfaces/core/trunk/api/src/main/java/javax/faces/FactoryFinder.java?rev=644540&r1=644539&r2=644540&view=diff
==============================================================================
--- myfaces/core/trunk/api/src/main/java/javax/faces/FactoryFinder.java (original)
+++ myfaces/core/trunk/api/src/main/java/javax/faces/FactoryFinder.java Thu Apr 3 15:49:22 2008
@@ -40,6 +40,10 @@
public static final String LIFECYCLE_FACTORY = "javax.faces.lifecycle.LifecycleFactory";
public static final String RENDER_KIT_FACTORY = "javax.faces.render.RenderKitFactory";
+ /**
+ * used as a monitor for itself and _factories.
+ * Maps in this map are used as monitors for themselves and the corresponding maps in _factories.
+ */
private static Map _registeredFactoryNames = new HashMap();
/**
* Maps from classLoader to another map, the container (i.e. Tomcat) will create a class loader for
@@ -81,9 +85,12 @@
//This code must be synchronized because this could cause a problem when
//using update feature each time of myfaces (org.apache.myfaces.CONFIG_REFRESH_PERIOD)
//In this moment, a concurrency problem could happen
+ Map factoryClassNames = null;
+ Map factoryMap = null;
+
synchronized(_registeredFactoryNames)
{
- Map factoryClassNames = (Map) _registeredFactoryNames.get(classLoader);
+ factoryClassNames = (Map) _registeredFactoryNames.get(classLoader);
if (factoryClassNames == null)
{
@@ -103,29 +110,44 @@
throw new IllegalStateException(message);
}
- if (! factoryClassNames.containsKey(factoryName)) {
+ if (! factoryClassNames.containsKey(factoryName))
+ {
throw new IllegalArgumentException("no factory " + factoryName + " configured for this application.");
}
- Map factoryMap = (Map) _factories.get(classLoader);
+ factoryMap = (Map) _factories.get(classLoader);
- if (factoryMap == null) {
+ if (factoryMap == null)
+ {
factoryMap = new HashMap();
_factories.put(classLoader, factoryMap);
}
- Object factory = factoryMap.get(factoryName);
-
- if (factory == null) {
- List classNames = (List) factoryClassNames.get(factoryName);
- factory = newFactoryInstance((Class)ABSTRACT_FACTORY_CLASSES.get(factoryName), classNames.iterator(), classLoader);
- factoryMap.put(factoryName, factory);
- return factory;
- }
- else
+ }
+
+ List classNames = null;
+ Object factory = null;
+ synchronized (factoryClassNames)
+ {
+ factory = factoryMap.get(factoryName);
+ if (factory != null)
{
return factory;
}
+ classNames = (List) factoryClassNames.get(factoryName);
}
+
+ //release lock while calling out
+ factory = newFactoryInstance((Class)ABSTRACT_FACTORY_CLASSES.get(factoryName), classNames.iterator(), classLoader);
+
+ synchronized (factoryClassNames)
+ {
+ //check if someone else already installed the factory
+ if (factoryMap.get(factoryName) == null)
+ {
+ factoryMap.put(factoryName, factory);
+ }
+ }
+ return factory;
}
@@ -199,6 +221,7 @@
checkFactoryName(factoryName);
ClassLoader classLoader = getClassLoader();
+ Map factoryClassNames = null;
synchronized(_registeredFactoryNames)
{
Map factories = (Map) _factories.get(classLoader);
@@ -209,14 +232,16 @@
return;
}
- Map factoryClassNames = (Map) _registeredFactoryNames.get(classLoader);
+ factoryClassNames = (Map) _registeredFactoryNames.get(classLoader);
if (factoryClassNames == null)
{
factoryClassNames = new HashMap();
_registeredFactoryNames.put(classLoader, factoryClassNames);
}
-
+ }
+ synchronized (factoryClassNames)
+ {
List classNameList = (List) factoryClassNames.get(factoryName);
if (classNameList == null) {
@@ -240,7 +265,7 @@
// _registeredFactoryNames has as value type Map<String,List> and this must
//be cleaned before release (for gc).
- Map factoryClassNames = _registeredFactoryNames.get(classLoader);
+ Map factoryClassNames = (Map) _registeredFactoryNames.get(classLoader);
if (factoryClassNames != null) factoryClassNames.clear();
_registeredFactoryNames.remove(classLoader);
}
@@ -248,7 +273,8 @@
private static void checkFactoryName(String factoryName)
{
- if (! VALID_FACTORY_NAMES.contains(factoryName)) {
+ if (! VALID_FACTORY_NAMES.contains(factoryName))
+ {
throw new IllegalArgumentException("factoryName '" + factoryName + "'");
}
}