mirror of https://github.com/apache/lucene.git
Additional thread safety around filter creation - old code could create duplicate CachingWrapperFilter if thread1 gets cache miss and thread 2 has a cache miss before thread1 populates cache with new CachingWrapperFilter.
Synchronization cost around whole method is OK here because Filter object construction should be a lightweight call. Note: CachingWrapperFilter currently has a similar bug in bits() method but adding "synchronized" around that whole method would not be a solution there because of the cost of evaluating filter.bits and the unnecessary blocking effect this would have on threads using different readers to the thread with the lock. git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@628921 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
caed55ef5e
commit
37a060c15c
|
@ -66,7 +66,7 @@ public class CachedFilterBuilder implements FilterBuilder {
|
|||
this.cacheSize=cacheSize;
|
||||
}
|
||||
|
||||
public Filter getFilter(Element e) throws ParserException
|
||||
public synchronized Filter getFilter(Element e) throws ParserException
|
||||
{
|
||||
|
||||
Element childElement = DOMUtils.getFirstChildOrFail(e);
|
||||
|
@ -78,8 +78,7 @@ public class CachedFilterBuilder implements FilterBuilder {
|
|||
|
||||
// Test to see if child Element is a query or filter that needs to be
|
||||
// cached
|
||||
QueryBuilder qb = queryFactory.getQueryBuilder(childElement
|
||||
.getNodeName());
|
||||
QueryBuilder qb = queryFactory.getQueryBuilder(childElement.getNodeName());
|
||||
Object cacheKey = null;
|
||||
Query q = null;
|
||||
Filter f = null;
|
||||
|
@ -92,15 +91,11 @@ public class CachedFilterBuilder implements FilterBuilder {
|
|||
f = filterFactory.getFilter(childElement);
|
||||
cacheKey = f;
|
||||
}
|
||||
Filter cachedFilter = null;
|
||||
synchronized (filterCache)
|
||||
{ // check cache
|
||||
cachedFilter = (Filter) filterCache.get(cacheKey);
|
||||
Filter cachedFilter = (Filter) filterCache.get(cacheKey);
|
||||
if (cachedFilter != null)
|
||||
{
|
||||
return cachedFilter; // cache hit
|
||||
}
|
||||
}
|
||||
|
||||
//cache miss
|
||||
if (qb != null)
|
||||
|
@ -111,10 +106,7 @@ public class CachedFilterBuilder implements FilterBuilder {
|
|||
cachedFilter = new CachingWrapperFilter(f);
|
||||
}
|
||||
|
||||
synchronized (filterCache)
|
||||
{ // update cache
|
||||
filterCache.put(cacheKey, cachedFilter);
|
||||
}
|
||||
return cachedFilter;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue