LUCENE-3541: remove IndexInput.copyBuf

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1190410 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -647,6 +647,10 @@ Changes in backwards compatibility policy
   (instead of the old reader) if there are no changes in the index, to
   prevent the common pitfall of accidentally closing the old reader.
 
+* LUCENE-3541: Remove IndexInput's protected copyBuf. If you want to
+  keep a buffer in your IndexInput, do this yourself in your implementation, 
+  and be sure to do the right thing on clone()!  (Robert Muir)
+
 Changes in runtime behavior
 
 * LUCENE-3520: IndexReader.openIfChanged, when passed a near-real-time
@@ -699,6 +703,10 @@ Bug fixes
   error message in IndexFormatTooOldException was incorrect. (Uwe Schindler,
   Mike McCandless)
 
+* LUCENE-3541: IndexInput's default copyBytes() implementation was not safe 
+  across multiple threads, because all clones shared the same buffer.
+  (Robert Muir)
+
 New Features
 
 * LUCENE-3448: Added FixedBitSet.and(other/DISI), andNot(other/DISI).

lucene/src/java/org/apache/lucene/store/IndexInput.java

@@ -26,8 +26,6 @@ import java.io.IOException;
  */
 public abstract class IndexInput extends DataInput implements Cloneable,Closeable {
 
-  protected byte[] copyBuf = null;
-
   /** Closes the stream to further operations. */
   public abstract void close() throws IOException;
 
@@ -59,9 +57,7 @@ public abstract class IndexInput extends DataInput implements Cloneable,Closeabl
   public void copyBytes(IndexOutput out, long numBytes) throws IOException {
     assert numBytes >= 0: "numBytes=" + numBytes;
 
-    if (copyBuf == null) {
-      copyBuf = new byte[BufferedIndexInput.BUFFER_SIZE];
-    }
+    byte copyBuf[] = new byte[BufferedIndexInput.BUFFER_SIZE];
 
     while (numBytes > 0) {
       final int toCopy = (int) (numBytes > copyBuf.length ? copyBuf.length : numBytes);

lucene/src/test/org/apache/lucene/store/TestCopyBytes.java

@@ -17,6 +17,8 @@ package org.apache.lucene.store;
  * limitations under the License.
  */
 
+import java.io.IOException;
+
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util._TestUtil;
 
@@ -104,4 +106,68 @@ public class TestCopyBytes extends LuceneTestCase {
       dir.close();
     }
   }
+  
+  // LUCENE-3541
+  public void testCopyBytesWithThreads() throws Exception {
+    int datalen = _TestUtil.nextInt(random, 101, 10000);
+    byte data[] = new byte[datalen];
+    random.nextBytes(data);
+    
+    Directory d = newDirectory();
+    IndexOutput output = d.createOutput("data", IOContext.DEFAULT);
+    output.writeBytes(data, 0, datalen);
+    output.close();
+    
+    IndexInput input = d.openInput("data", IOContext.DEFAULT);
+    IndexOutput outputHeader = d.createOutput("header", IOContext.DEFAULT);
+    // copy our 100-byte header
+    input.copyBytes(outputHeader, 100);
+    outputHeader.close();
+    
+    // now make N copies of the remaining bytes
+    CopyThread copies[] = new CopyThread[10];
+    for (int i = 0; i < copies.length; i++) {
+      copies[i] = new CopyThread((IndexInput) input.clone(), d.createOutput("copy" + i, IOContext.DEFAULT));
+    }
+    
+    for (int i = 0; i < copies.length; i++) {
+      copies[i].start();
+    }
+    
+    for (int i = 0; i < copies.length; i++) {
+      copies[i].join();
+    }
+    
+    for (int i = 0; i < copies.length; i++) {
+      IndexInput copiedData = d.openInput("copy" + i, IOContext.DEFAULT);
+      byte[] dataCopy = new byte[datalen];
+      System.arraycopy(data, 0, dataCopy, 0, 100); // copy the header for easy testing
+      copiedData.readBytes(dataCopy, 100, datalen-100);
+      assertArrayEquals(data, dataCopy);
+      copiedData.close();
+    }
+    input.close();
+    d.close();
+    
+  }
+  
+  static class CopyThread extends Thread {
+    final IndexInput src;
+    final IndexOutput dst;
+    
+    CopyThread(IndexInput src, IndexOutput dst) {
+      this.src = src;
+      this.dst = dst;
+    }
+
+    @Override
+    public void run() {
+      try {
+        src.copyBytes(dst, src.length()-100);
+        dst.close();
+      } catch (IOException ex) {
+        throw new RuntimeException(ex);
+      }
+    }
+  }
 }