You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2015/08/21 17:26:55 UTC
svn commit: r1697011 - in /lucene/dev/branches/branch_5x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/store/ lucene/sandbox/
lucene/sandbox/src/java/org/apache/lucene/bkdtree/
lucene/sandbox/src/java/org/apache/lucene/rangetree/ luce...
Author: mikemccand
Date: Fri Aug 21 15:26:54 2015
New Revision: 1697011
URL: http://svn.apache.org/r1697011
Log:
LUCENE-6745: IndexInput.clone is not thread-safe; fix BKD/RangeTree to respect that
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/lucene/ (props changed)
lucene/dev/branches/branch_5x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/lucene/core/ (props changed)
lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/IndexInput.java
lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/RAMInputStream.java
lucene/dev/branches/branch_5x/lucene/sandbox/ (props changed)
lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/bkdtree/BKDTreeDocValuesProducer.java
lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/rangetree/RangeTreeDocValuesProducer.java
lucene/dev/branches/branch_5x/lucene/test-framework/ (props changed)
lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java
Modified: lucene/dev/branches/branch_5x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/CHANGES.txt?rev=1697011&r1=1697010&r2=1697011&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/lucene/CHANGES.txt Fri Aug 21 15:26:54 2015
@@ -36,8 +36,6 @@ Bug Fixes
test data. In addition, the performance of those filters was improved
significantly. (Uwe Schindler, Robert Muir)
-* LUCENE-6745: RAMInputStream.clone was not thread safe (Mike McCandless)
-
* LUCENE-6748: UsageTrackingQueryCachingPolicy no longer caches trivial queries
like MatchAllDocsQuery. (Adrien Grand)
Modified: lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/IndexInput.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/IndexInput.java?rev=1697011&r1=1697010&r2=1697011&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/IndexInput.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/IndexInput.java Fri Aug 21 15:26:54 2015
@@ -83,6 +83,10 @@ public abstract class IndexInput extends
*
* <p>If you access the cloned IndexInput after closing the original object,
* any <code>readXXX</code> methods will throw {@link AlreadyClosedException}.
+ *
+ * <p>This method is NOT thread safe, so if the current {@code IndexInput}
+ * is being used by one thread while {@code clone} is called by another,
+ * disaster could strike.
*/
@Override
public IndexInput clone() {
Modified: lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/RAMInputStream.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/RAMInputStream.java?rev=1697011&r1=1697010&r2=1697011&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/RAMInputStream.java (original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/java/org/apache/lucene/store/RAMInputStream.java Fri Aug 21 15:26:54 2015
@@ -157,19 +157,4 @@ public class RAMInputStream extends Inde
}
};
}
-
- @Override
- public RAMInputStream clone() {
- RAMInputStream clone = (RAMInputStream) super.clone();
- // If another thread was using our instance, this new clone could have a mismatched currentBuffer and currentBufferIndex, so we do
- // a "fresh seek" here:
- clone.currentBuffer = null;
- try {
- clone.seek(getFilePointer());
- } catch (IOException ioe) {
- // Should not happen!
- throw new AssertionError(ioe);
- }
- return clone;
- }
}
Modified: lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/bkdtree/BKDTreeDocValuesProducer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/bkdtree/BKDTreeDocValuesProducer.java?rev=1697011&r1=1697010&r2=1697011&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/bkdtree/BKDTreeDocValuesProducer.java (original)
+++ lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/bkdtree/BKDTreeDocValuesProducer.java Fri Aug 21 15:26:54 2015
@@ -99,8 +99,13 @@ class BKDTreeDocValuesProducer extends D
if (fp == null) {
throw new IllegalArgumentException("this field was not indexed as a BKDPointField");
}
- datIn.seek(fp);
- treeReader = new BKDTreeReader(datIn, maxDoc);
+
+ // LUCENE-6697: never do real IOPs with the original IndexInput because search
+ // threads can be concurrently cloning it:
+ IndexInput clone = datIn.clone();
+
+ clone.seek(fp);
+ treeReader = new BKDTreeReader(clone, maxDoc);
// Only hang onto the reader when we are not merging:
if (merging == false) {
Modified: lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/rangetree/RangeTreeDocValuesProducer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/rangetree/RangeTreeDocValuesProducer.java?rev=1697011&r1=1697010&r2=1697011&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/rangetree/RangeTreeDocValuesProducer.java (original)
+++ lucene/dev/branches/branch_5x/lucene/sandbox/src/java/org/apache/lucene/rangetree/RangeTreeDocValuesProducer.java Fri Aug 21 15:26:54 2015
@@ -99,8 +99,12 @@ class RangeTreeDocValuesProducer extends
// FieldInfos checks has already ensured we are a DV field of this type, and Codec ensures
// this DVFormat was used at write time:
assert fp != null;
- datIn.seek(fp);
- treeReader = new RangeTreeReader(datIn);
+
+ // LUCENE-6697: never do real IOPs with the original IndexInput because search
+ // threads can be concurrently cloning it:
+ IndexInput clone = datIn.clone();
+ clone.seek(fp);
+ treeReader = new RangeTreeReader(clone);
// Only hang onto the reader when we are not merging:
if (merging == false) {
@@ -148,9 +152,11 @@ class RangeTreeDocValuesProducer extends
// this DVFormat was used at write time:
assert fp != null;
- datIn.seek(fp);
- //System.out.println("load field=" + field.name);
- treeReader = new RangeTreeReader(datIn);
+ // LUCENE-6697: never do real IOPs with the original IndexInput because search
+ // threads can be concurrently cloning it:
+ IndexInput clone = datIn.clone();
+ clone.seek(fp);
+ treeReader = new RangeTreeReader(clone);
// Only hang onto the reader when we are not merging:
if (merging == false) {
Modified: lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java?rev=1697011&r1=1697010&r2=1697011&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java (original)
+++ lucene/dev/branches/branch_5x/lucene/test-framework/src/java/org/apache/lucene/store/BaseDirectoryTestCase.java Fri Aug 21 15:26:54 2015
@@ -1167,59 +1167,4 @@ public abstract class BaseDirectoryTestC
in.close(); // close again
dir.close();
}
-
- public void testCloneThreadSafety() throws Exception {
- Directory dir = getDirectory(createTempDir());
- IndexOutput out = dir.createOutput("randombytes", IOContext.DEFAULT);
-
- // Write file with at least 20 K random bytes:
- final int numBytes = atLeast(20*1024);
- final byte[] bytes = new byte[numBytes];
- random().nextBytes(bytes);
- out.writeBytes(bytes, 0, bytes.length);
- out.close();
-
- // Then read the bytes back at random seek points from multiple threads:
- final IndexInput in = dir.openInput("randombytes", IOContext.DEFAULT);
-
- int numThreads = 4;
- Thread[] threads = new Thread[numThreads];
- for(int i=0;i<numThreads;i++) {
- final int finalI = i;
- threads[i] = new Thread() {
- @Override
- public void run() {
- int numIters = atLeast(1000);
- byte[] scratch = new byte[numBytes];
- for(int iter=0;iter<numIters;iter++) {
- // First thread uses the original IndexInput, all other threads use clone:
- IndexInput myIn;
- if (finalI == 0) {
- myIn = in;
- } else {
- myIn = in.clone();
- }
- int spot = random().nextInt(numBytes/2);
- try {
- myIn.seek(spot);
- int length = numBytes-spot;
- myIn.readBytes(scratch, 0, length);
- for(int i=0;i<length;i++) {
- assertEquals(bytes[spot+i], scratch[i]);
- }
- } catch (IOException ioe) {
- throw new RuntimeException(ioe);
- }
- }
- }
- };
- threads[i].start();
- }
-
- for(Thread thread : threads) {
- thread.join();
- }
- in.close();
- dir.close();
- }
}