You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by pa...@apache.org on 2007/08/15 22:11:51 UTC

svn commit: r566323 - /felix/trunk/framework/src/main/java/org/apache/felix/framework/FilterImpl.java

Author: pauls
Date: Wed Aug 15 13:11:50 2007
New Revision: 566323

URL: http://svn.apache.org/viewvc?view=rev&rev=566323
Log:
Fix a bug in the Framework FilterImpl which makes it not thread safe on execution (FELIX-338).

Modified:
    felix/trunk/framework/src/main/java/org/apache/felix/framework/FilterImpl.java

Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/FilterImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/FilterImpl.java?view=diff&rev=566323&r1=566322&r2=566323
==============================================================================
--- felix/trunk/framework/src/main/java/org/apache/felix/framework/FilterImpl.java (original)
+++ felix/trunk/framework/src/main/java/org/apache/felix/framework/FilterImpl.java Wed Aug 15 13:11:50 2007
@@ -21,6 +21,7 @@
 import java.io.CharArrayReader;
 import java.io.IOException;
 import java.util.*;
+import java.lang.ref.SoftReference;
 
 import org.apache.felix.framework.util.StringMap;
 import org.apache.felix.framework.util.ldap.*;
@@ -34,10 +35,10 @@
 **/
 public class FilterImpl implements Filter
 {
-    private Logger m_logger = null;
-    private String m_toString = null;
-    private Evaluator m_evaluator = null;
-    private SimpleMapper m_mapper = null;
+    private final ThreadLocal m_cache = new ThreadLocal();
+    private final Logger m_logger;
+    private final Object[] m_program;
+    private volatile String m_toString;
 
 // TODO: FilterImpl needs a logger, this is a hack for FrameworkUtil.
     public FilterImpl(String expr) throws InvalidSyntaxException
@@ -57,32 +58,28 @@
             throw new InvalidSyntaxException("Filter cannot be null", null);
         }
 
-        if (expr != null)
+        CharArrayReader car = new CharArrayReader(expr.toCharArray());
+        LdapLexer lexer = new LdapLexer(car);
+        Parser parser = new Parser(lexer);
+        try
         {
-            CharArrayReader car = new CharArrayReader(expr.toCharArray());
-            LdapLexer lexer = new LdapLexer(car);
-            Parser parser = new Parser(lexer);
-            try
-            {
-                if (!parser.start())
-                {
-                    throw new InvalidSyntaxException(
-                        "Failed to parse LDAP query.", expr);
-                }
-            }
-            catch (ParseException ex)
-            {
-                throw new InvalidSyntaxException(
-                    ex.getMessage(), expr);
-            }
-            catch (IOException ex)
+            if (!parser.start())
             {
                 throw new InvalidSyntaxException(
-                    ex.getMessage(), expr);
+                    "Failed to parse LDAP query.", expr);
             }
-            m_evaluator = new Evaluator(parser.getProgram());
-            m_mapper = new SimpleMapper();
         }
+        catch (ParseException ex)
+        {
+            throw new InvalidSyntaxException(
+               ex.getMessage(), expr);
+        }
+        catch (IOException ex)
+        {
+            throw new InvalidSyntaxException(
+                ex.getMessage(), expr);
+        }
+        m_program = parser.getProgram();
     }
 
     /**
@@ -114,29 +111,43 @@
         return toString().hashCode();
     }
 
-    /**
-     * Filter using a <tt>Dictionary</tt> object. The <tt>Filter</tt>
-     * is executed using the <tt>Dictionary</tt> object's keys and values.
-     * @param dict the <tt>Dictionary</tt> object whose keys and values
-     *             are used to determine a match.
-     * @return <tt>true</tt> if the <tt>Dictionary</tt> object's keys
-     *         and values match this filter; <tt>false</tt> otherwise.
-     * @throws IllegalArgumentException if the dictionary contains case
-     *         variants of the same key name.
-    **/
-    public boolean match(Dictionary dict)
+    private boolean match(Dictionary dict, ServiceReference ref, boolean caseSensitive) 
         throws IllegalArgumentException
     {
-        try
+        SoftReference tupleRef = (SoftReference) m_cache.get();
+        Evaluator evaluator = null;
+        SimpleMapper mapper = null;
+        Object[] tuple = null;
+
+        if (tupleRef != null) 
+        {
+            tuple = (Object[]) tupleRef.get();
+        }
+
+        if (tuple == null) 
         {
-            // Since the mapper instance is reused, we should
-            // null the source after use to avoid potential
-            // garbage collection issues.
-            m_mapper.setSource(dict, false);
-            boolean result = m_evaluator.evaluate(m_mapper);
-            m_mapper.setSource(null, false);
-            return result;
+            evaluator = new Evaluator(m_program);
+            mapper = new SimpleMapper();
         }
+        else 
+        {
+            evaluator = (Evaluator) tuple[0];
+            mapper = (SimpleMapper) tuple[1];
+        }
+
+        try 
+        {
+            if (dict != null) 
+            {
+                mapper.setSource(dict, caseSensitive);
+            }
+            else 
+            {
+                mapper.setSource(ref);
+            }
+
+            return evaluator.evaluate(mapper);
+        } 
         catch (AttributeNotFoundException ex)
         {
             log(Logger.LOG_DEBUG, "FilterImpl: Attribute not found.", ex);
@@ -145,10 +156,43 @@
         {
             log(Logger.LOG_ERROR, "FilterImpl: " + toString(), ex);
         }
+        finally 
+        {
+            if (dict != null) 
+            {
+                mapper.setSource(null, caseSensitive);
+            }
+            else 
+            {
+                mapper.setSource(null);
+            }
+            
+            if (tuple == null) 
+            {
+                m_cache.set(new SoftReference(new Object[] {evaluator, mapper}));
+            }
+        }
+
         return false;
     }
 
     /**
+     * Filter using a <tt>Dictionary</tt> object. The <tt>Filter</tt>
+     * is executed using the <tt>Dictionary</tt> object's keys and values.
+     * @param dict the <tt>Dictionary</tt> object whose keys and values
+     *             are used to determine a match.
+     * @return <tt>true</tt> if the <tt>Dictionary</tt> object's keys
+     *         and values match this filter; <tt>false</tt> otherwise.
+     * @throws IllegalArgumentException if the dictionary contains case
+     *         variants of the same key name.
+    **/
+    public boolean match(Dictionary dict)
+        throws IllegalArgumentException
+    {
+        return match(dict, null, false);
+    }
+
+    /**
      * Filter using a service's properties. The <tt>Filter</tt>
      * is executed using the properties of the referenced service.
      * @param ref A reference to the service whose properties
@@ -158,48 +202,12 @@
     **/
     public boolean match(ServiceReference ref)
     {
-        try
-        {
-            // Since the mapper instance is reused, we should
-            // null the source after use to avoid potential
-            // garbage collection issues.
-            m_mapper.setSource(ref);
-            boolean result = m_evaluator.evaluate(m_mapper);
-            m_mapper.setSource(null);
-            return result;
-        }
-        catch (AttributeNotFoundException ex)
-        {
-            log(Logger.LOG_DEBUG, "FilterImpl: Attribute not found.", ex);
-        }
-        catch (EvaluationException ex)
-        {
-            log(Logger.LOG_ERROR, "FilterImpl: " + toString(), ex);
-        }
-        return false;
+        return match(null, ref, false);
     }
 
     public boolean matchCase(Dictionary dict)
     {
-        try
-        {
-            // Since the mapper instance is reused, we should
-            // null the source after use to avoid potential
-            // garbage collection issues.
-            m_mapper.setSource(dict, true);
-            boolean result = m_evaluator.evaluate(m_mapper);
-            m_mapper.setSource(null, true);
-            return result;
-        }
-        catch (AttributeNotFoundException ex)
-        {
-            log(Logger.LOG_DEBUG, "FilterImpl: Attribute not found.", ex);
-        }
-        catch (EvaluationException ex)
-        {
-            log(Logger.LOG_ERROR, "FilterImpl: " + toString(), ex);
-        }
-        return false;
+        return match(dict, null, true);
     }
 
     /**
@@ -210,11 +218,23 @@
     {
         if (m_toString == null)
         {
-            m_toString = m_evaluator.toStringInfix();
+            m_toString = new Evaluator(m_program).toStringInfix();
         }
         return m_toString;
     }
 
+    private void log(int flag, String msg, Throwable th)
+    {
+        if (m_logger == null)
+        {
+            System.out.println(msg + ": " + th);
+        }
+        else
+        {
+            m_logger.log(flag, msg, th);
+        }
+    }
+
     static class SimpleMapper implements Mapper
     {
         private ServiceReference m_ref = null;
@@ -270,18 +290,6 @@
                 return m_ref.getProperty(name);
             }
             return m_map.get(name);
-        }
-    }
-
-    private void log(int flag, String msg, Throwable th)
-    {
-        if (m_logger == null)
-        {
-            System.out.println(msg + ": " + th);
-        }
-        else
-        {
-            m_logger.log(flag, msg, th);
         }
     }
 }