On 7 June 2011 21:49, <mbenson@apache.org> wrote:
> Author: mbenson
> Date: Tue Jun 7 20:49:04 2011
> New Revision: 1133155
>
> URL: http://svn.apache.org/viewvc?rev=1133155&view=rev
> Log:
> [JXPATH-141] FunctionLibrary Multithreading issue
I don't think this is fully thread-safe.
Although the byNamespace Map is created under a lock, another thread
may bypass the synch. block.
For non-final fields to be published correctly, both the writer and
reader threads need to use the same lock.
I suggest removing the double-check on the lock, i.e. just make the
functionCache() method synchronised.
If the map has already been created, the code path is very short.
> Modified:
> commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/FunctionLibrary.java
>
> Modified: commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/FunctionLibrary.java
> URL: http://svn.apache.org/viewvc/commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/FunctionLibrary.java?rev=1133155&r1=1133154&r2=1133155&view=diff
> ==============================================================================
> --- commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/FunctionLibrary.java
(original)
> +++ commons/proper/jxpath/trunk/src/java/org/apache/commons/jxpath/FunctionLibrary.java
Tue Jun 7 20:49:04 2011
> @@ -20,6 +20,7 @@ import java.util.ArrayList;
> import java.util.HashMap;
> import java.util.Iterator;
> import java.util.List;
> +import java.util.Map;
> import java.util.Set;
>
> /**
> @@ -32,8 +33,8 @@ import java.util.Set;
> * @version $Revision$ $Date$
> */
> public class FunctionLibrary implements Functions {
> - private List allFunctions = new ArrayList();
> - private HashMap byNamespace = null;
> + private final List allFunctions = new ArrayList();
> + private Map byNamespace;
>
> /**
> * Add functions to the library
> @@ -59,10 +60,7 @@ public class FunctionLibrary implements
> * @return Set<String>
> */
> public Set getUsedNamespaces() {
> - if (byNamespace == null) {
> - prepareCache();
> - }
> - return byNamespace.keySet();
> + return functionCache().keySet();
> }
>
> /**
> @@ -75,10 +73,7 @@ public class FunctionLibrary implements
> */
> public Function getFunction(String namespace, String name,
> Object[] parameters) {
> - if (byNamespace == null) {
> - prepareCache();
> - }
> - Object candidates = byNamespace.get(namespace);
> + Object candidates = functionCache().get(namespace);
> if (candidates instanceof Functions) {
> return ((Functions) candidates).getFunction(
> namespace,
> @@ -105,28 +100,34 @@ public class FunctionLibrary implements
> /**
> * Prepare the cache.
> */
> - private void prepareCache() {
> - byNamespace = new HashMap();
> - int count = allFunctions.size();
> - for (int i = 0; i < count; i++) {
> - Functions funcs = (Functions) allFunctions.get(i);
> - Set namespaces = funcs.getUsedNamespaces();
> - for (Iterator it = namespaces.iterator(); it.hasNext();) {
> - String ns = (String) it.next();
> - Object candidates = byNamespace.get(ns);
> - if (candidates == null) {
> - byNamespace.put(ns, funcs);
> - }
> - else if (candidates instanceof Functions) {
> - List lst = new ArrayList();
> - lst.add(candidates);
> - lst.add(funcs);
> - byNamespace.put(ns, lst);
> - }
> - else {
> - ((List) candidates).add(funcs);
> + private Map functionCache() {
> + if (byNamespace == null) {
> + synchronized (this) {
> + //read again
> + if (byNamespace == null) {
> + byNamespace = new HashMap();
> + int count = allFunctions.size();
> + for (int i = 0; i < count; i++) {
> + Functions funcs = (Functions) allFunctions.get(i);
> + Set namespaces = funcs.getUsedNamespaces();
> + for (Iterator it = namespaces.iterator(); it.hasNext();)
{
> + String ns = (String) it.next();
> + Object candidates = byNamespace.get(ns);
> + if (candidates == null) {
> + byNamespace.put(ns, funcs);
> + } else if (candidates instanceof Functions)
{
> + List lst = new ArrayList();
> + lst.add(candidates);
> + lst.add(funcs);
> + byNamespace.put(ns, lst);
> + } else {
> + ((List) candidates).add(funcs);
> + }
> + }
> + }
> }
> }
> }
> + return byNamespace;
> }
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
|