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); } } }