You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2014/11/26 15:26:43 UTC

svn commit: r1641822 - /lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java

Author: jpountz
Date: Wed Nov 26 14:26:43 2014
New Revision: 1641822

URL: http://svn.apache.org/r1641822
Log:
LUCENE-6076: Fix locking in CachingWrapperFilter.getChildResources.

Modified:
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java?rev=1641822&r1=1641821&r2=1641822&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java Wed Nov 26 14:26:43 2014
@@ -22,6 +22,7 @@ import static org.apache.lucene.search.D
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.WeakHashMap;
@@ -146,8 +147,12 @@ public class CachingWrapperFilter extend
   }
 
   @Override
-  public synchronized Iterable<? extends Accountable> getChildResources() {
-    // Sync only to pull the current set of values:
-    return Accountables.namedAccountables("segment", cache);
+  public Iterable<? extends Accountable> getChildResources() {
+    // Sync to pull the current set of values:
+    final Map<Object, DocIdSet> copy;
+    synchronized (cache) {
+      copy = new HashMap<>(cache);
+    }
+    return Accountables.namedAccountables("segment", copy);
   }
 }



Re: svn commit: r1641822 - /lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java

Posted by Adrien Grand <jp...@gmail.com>.
I just committed a fix. Thanks for the note!

On Wed, Nov 26, 2014 at 3:32 PM, Robert Muir <rc...@gmail.com> wrote:
> Accountables* methods are always a snapshot in time, so we don't need
> to make a copy here.
>
> On Wed, Nov 26, 2014 at 9:26 AM,  <jp...@apache.org> wrote:
>> Author: jpountz
>> Date: Wed Nov 26 14:26:43 2014
>> New Revision: 1641822
>>
>> URL: http://svn.apache.org/r1641822
>> Log:
>> LUCENE-6076: Fix locking in CachingWrapperFilter.getChildResources.
>>
>> Modified:
>>     lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java
>>
>> Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java
>> URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java?rev=1641822&r1=1641821&r2=1641822&view=diff
>> ==============================================================================
>> --- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java (original)
>> +++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java Wed Nov 26 14:26:43 2014
>> @@ -22,6 +22,7 @@ import static org.apache.lucene.search.D
>>  import java.io.IOException;
>>  import java.util.ArrayList;
>>  import java.util.Collections;
>> +import java.util.HashMap;
>>  import java.util.List;
>>  import java.util.Map;
>>  import java.util.WeakHashMap;
>> @@ -146,8 +147,12 @@ public class CachingWrapperFilter extend
>>    }
>>
>>    @Override
>> -  public synchronized Iterable<? extends Accountable> getChildResources() {
>> -    // Sync only to pull the current set of values:
>> -    return Accountables.namedAccountables("segment", cache);
>> +  public Iterable<? extends Accountable> getChildResources() {
>> +    // Sync to pull the current set of values:
>> +    final Map<Object, DocIdSet> copy;
>> +    synchronized (cache) {
>> +      copy = new HashMap<>(cache);
>> +    }
>> +    return Accountables.namedAccountables("segment", copy);
>>    }
>>  }
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>



-- 
Adrien

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Re: svn commit: r1641822 - /lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java

Posted by Robert Muir <rc...@gmail.com>.
Accountables* methods are always a snapshot in time, so we don't need
to make a copy here.

On Wed, Nov 26, 2014 at 9:26 AM,  <jp...@apache.org> wrote:
> Author: jpountz
> Date: Wed Nov 26 14:26:43 2014
> New Revision: 1641822
>
> URL: http://svn.apache.org/r1641822
> Log:
> LUCENE-6076: Fix locking in CachingWrapperFilter.getChildResources.
>
> Modified:
>     lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java
>
> Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java
> URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java?rev=1641822&r1=1641821&r2=1641822&view=diff
> ==============================================================================
> --- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java (original)
> +++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/search/CachingWrapperFilter.java Wed Nov 26 14:26:43 2014
> @@ -22,6 +22,7 @@ import static org.apache.lucene.search.D
>  import java.io.IOException;
>  import java.util.ArrayList;
>  import java.util.Collections;
> +import java.util.HashMap;
>  import java.util.List;
>  import java.util.Map;
>  import java.util.WeakHashMap;
> @@ -146,8 +147,12 @@ public class CachingWrapperFilter extend
>    }
>
>    @Override
> -  public synchronized Iterable<? extends Accountable> getChildResources() {
> -    // Sync only to pull the current set of values:
> -    return Accountables.namedAccountables("segment", cache);
> +  public Iterable<? extends Accountable> getChildResources() {
> +    // Sync to pull the current set of values:
> +    final Map<Object, DocIdSet> copy;
> +    synchronized (cache) {
> +      copy = new HashMap<>(cache);
> +    }
> +    return Accountables.namedAccountables("segment", copy);
>    }
>  }
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org