lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mikemcc...@apache.org
Subject svn commit: r1726056 - in /lucene/dev/branches/branch_5x: ./ lucene/ lucene/core/ lucene/core/src/java/org/apache/lucene/store/ lucene/core/src/test/org/apache/lucene/store/ lucene/test-framework/ lucene/test-framework/src/java/org/apache/lucene/store/
Date Thu, 21 Jan 2016 18:40:03 GMT
Author: mikemccand
Date: Thu Jan 21 18:40:02 2016
New Revision: 1726056

URL: http://svn.apache.org/viewvc?rev=1726056&view=rev
Log:
LUCENE-6932: RAMInputStream now throws EOFException if you seek beyond the end of the file

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/core/src/test/org/apache/lucene/store/TestRAMDirectory.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=1726056&r1=1726055&r2=1726056&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/lucene/CHANGES.txt Thu Jan 21 18:40:02 2016
@@ -55,6 +55,9 @@ API Changes
 * LUCENE-6981: SpanQuery.getTermContexts() helper methods are now public, and
   SpanScorer has a public getSpans() method. (Alan Woodward)
 
+* LUCENE-6932: IndexInput.seek implementations now throw EOFException
+  if you seek beyond the end of the file (Adrien Grand, Mike McCandless)
+
 Optimizations
 
 * LUCENE-6951: Improve GeoPointInPolygonQuery using point orientation based
@@ -86,9 +89,9 @@ Bug Fixes
 * LUCENE-6976: BytesRefTermAttributeImpl.copyTo NPE'ed if BytesRef was null.
   Added equals & hashCode, and a new test for these things. (David Smiley)
 
-* LUCENE-6932: RAMDirectory's IndexInput should always throw
-  EOFException if you seek past the end of the file and then try to
-  read (Stéphane Campinas via Mike McCandless)
+* LUCENE-6932: RAMDirectory's IndexInput was failing to throw
+  EOFException in some cases (Stéphane Campinas, Adrien Grand via Mike
+  McCandless)
 
 * LUCENE-6896: Don't treat the smallest possible norm value as an infinitely
   long document in SimilarityBase or BM25Similarity. Add more warnings to sims

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=1726056&r1=1726055&r2=1726056&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
Thu Jan 21 18:40:02 2016
@@ -63,7 +63,10 @@ public abstract class IndexInput extends
    */
   public abstract long getFilePointer();
 
-  /** Sets current position in this file, where the next read will occur.
+  /** Sets current position in this file, where the next read will occur.  If this is
+   *  beyond the end of the file then this will throw {@code EOFException} and then the
+   *  stream is in an undetermined state.
+   *
    * @see #getFilePointer()
    */
   public abstract void seek(long pos) throws IOException;

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=1726056&r1=1726055&r2=1726056&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
Thu Jan 21 18:40:02 2016
@@ -17,14 +17,15 @@ package org.apache.lucene.store;
  * limitations under the License.
  */
 
-import java.io.IOException;
 import java.io.EOFException;
+import java.io.IOException;
+
+import static org.apache.lucene.store.RAMOutputStream.BUFFER_SIZE;
 
 /** A memory-resident {@link IndexInput} implementation. 
  *  
  *  @lucene.internal */
 public class RAMInputStream extends IndexInput implements Cloneable {
-  static final int BUFFER_SIZE = RAMOutputStream.BUFFER_SIZE;
 
   private final RAMFile file;
   private final long length;
@@ -33,7 +34,6 @@ public class RAMInputStream extends Inde
   private int currentBufferIndex;
   
   private int bufferPosition;
-  private long bufferStart;
   private int bufferLength;
 
   public RAMInputStream(String name, RAMFile f) throws IOException {
@@ -48,10 +48,7 @@ public class RAMInputStream extends Inde
       throw new IOException("RAMInputStream too large length=" + length + ": " + name); 
     }
 
-    // make sure that we switch to the
-    // first needed buffer lazily
-    currentBufferIndex = -1;
-    currentBuffer = null;
+    setCurrentBuffer();
   }
 
   @Override
@@ -66,9 +63,8 @@ public class RAMInputStream extends Inde
 
   @Override
   public byte readByte() throws IOException {
-    if (bufferPosition >= bufferLength) {
-      currentBufferIndex++;
-      switchCurrentBuffer(true);
+    if (bufferPosition == bufferLength) {
+      nextBuffer();
     }
     return currentBuffer[bufferPosition++];
   }
@@ -76,9 +72,9 @@ public class RAMInputStream extends Inde
   @Override
   public void readBytes(byte[] b, int offset, int len) throws IOException {
     while (len > 0) {
-      if (bufferPosition >= bufferLength) {
-        currentBufferIndex++;
-        switchCurrentBuffer(true);
+
+      if (bufferPosition == bufferLength) {
+        nextBuffer();
       }
 
       int remainInBuffer = bufferLength - bufferPosition;
@@ -90,39 +86,49 @@ public class RAMInputStream extends Inde
     }
   }
 
-  private final void switchCurrentBuffer(boolean enforceEOF) throws IOException {
-    bufferStart = (long) BUFFER_SIZE * (long) currentBufferIndex;
-    if (bufferStart > length || currentBufferIndex >= file.numBuffers()) {
-      // end of file reached, no more buffers left
-      if (enforceEOF) {
-        throw new EOFException("read past EOF: " + this);
-      } else {
-        // Force EOF if a read later takes place at this position
-        currentBufferIndex--;
-        bufferPosition = BUFFER_SIZE;
-      }
-    } else {
-      currentBuffer = file.getBuffer(currentBufferIndex);
-      bufferPosition = 0;
-      long buflen = length - bufferStart;
-      bufferLength = buflen > BUFFER_SIZE ? BUFFER_SIZE : (int) buflen;
-    }
-  }
-
   @Override
   public long getFilePointer() {
-    return currentBufferIndex < 0 ? 0 : bufferStart + bufferPosition;
+    return (long) currentBufferIndex * BUFFER_SIZE + bufferPosition;
   }
 
   @Override
   public void seek(long pos) throws IOException {
-    if (currentBuffer == null || pos < bufferStart || pos >= bufferStart + BUFFER_SIZE)
{
-      currentBufferIndex = (int) (pos / BUFFER_SIZE);
-      switchCurrentBuffer(false);
-    }
-    if (pos < BUFFER_SIZE * (long) file.numBuffers()) {
-      // do not overwrite bufferPosition if EOF should be thrown on the next read
-      bufferPosition = (int) (pos % BUFFER_SIZE);
+    int newBufferIndex = (int) (pos / BUFFER_SIZE);
+
+    if (newBufferIndex != currentBufferIndex) {
+      // we seek'd to a different buffer:
+      currentBufferIndex = newBufferIndex;
+      setCurrentBuffer();
+    }
+
+    bufferPosition = (int) (pos % BUFFER_SIZE);
+
+    // This is not >= because seeking to exact end of file is OK: this is where
+    // you'd also be if you did a readBytes of all bytes in the file)
+    if (getFilePointer() > length()) {
+      throw new EOFException("read past EOF: pos=" + getFilePointer() + " vs length=" + length()
+ ": " + this);
+    }
+  }
+
+  private void nextBuffer() throws IOException {
+    // This is >= because we are called when there is at least 1 more byte to read:
+    if (getFilePointer() >= length()) {
+      throw new EOFException("read past EOF: pos=" + getFilePointer() + " vs length=" + length()
+ ": " + this);
+    }
+    currentBufferIndex++;
+    setCurrentBuffer();
+    assert currentBuffer != null;
+    bufferPosition = 0;
+  }
+
+  private final void setCurrentBuffer() throws IOException {
+    if (currentBufferIndex < file.numBuffers()) {
+      currentBuffer = file.getBuffer(currentBufferIndex);
+      assert currentBuffer != null;
+      long bufferStart = (long) BUFFER_SIZE * (long) currentBufferIndex;
+      bufferLength = (int) Math.min(BUFFER_SIZE, length - bufferStart);
+    } else {
+      currentBuffer = null;
     }
   }
 

Modified: lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/store/TestRAMDirectory.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/store/TestRAMDirectory.java?rev=1726056&r1=1726055&r2=1726056&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/store/TestRAMDirectory.java
(original)
+++ lucene/dev/branches/branch_5x/lucene/core/src/test/org/apache/lucene/store/TestRAMDirectory.java
Thu Jan 21 18:40:02 2016
@@ -21,7 +21,6 @@ import java.io.EOFException;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Random;
@@ -152,10 +151,12 @@ public class TestRAMDirectory extends Ba
         }
       };
     }
-    for (int i=0; i<numThreads; i++)
+    for (int i=0; i<numThreads; i++) {
       threads[i].start();
-    for (int i=0; i<numThreads; i++)
+    }
+    for (int i=0; i<numThreads; i++) {
       threads[i].join();
+    }
 
     writer.forceMerge(1);
     assertEquals(ramDir.sizeInBytes(), ramDir.getRecomputedSizeInBytes());
@@ -188,4 +189,5 @@ public class TestRAMDirectory extends Ba
         }
       }
     }
-  }}
+  }
+}

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=1726056&r1=1726055&r2=1726056&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
Thu Jan 21 18:40:02 2016
@@ -586,15 +586,15 @@ public abstract class BaseDirectoryTestC
     Directory dir = getDirectory(createTempDir("testSeekToEOFThenBack"));
 
     IndexOutput o = dir.createOutput("out", newIOContext(random()));
-    byte[] bytes = new byte[3*RAMInputStream.BUFFER_SIZE];
+    byte[] bytes = new byte[3*RAMOutputStream.BUFFER_SIZE];
     o.writeBytes(bytes, 0, bytes.length);
     o.close();
 
     IndexInput i = dir.openInput("out", newIOContext(random()));
-    i.seek(2*RAMInputStream.BUFFER_SIZE-1);
-    i.seek(3*RAMInputStream.BUFFER_SIZE);
-    i.seek(RAMInputStream.BUFFER_SIZE);
-    i.readBytes(bytes, 0, 2*RAMInputStream.BUFFER_SIZE);
+    i.seek(2*RAMOutputStream.BUFFER_SIZE-1);
+    i.seek(3*RAMOutputStream.BUFFER_SIZE);
+    i.seek(RAMOutputStream.BUFFER_SIZE);
+    i.readBytes(bytes, 0, 2*RAMOutputStream.BUFFER_SIZE);
     i.close();
     dir.close();
   }
@@ -1165,4 +1165,40 @@ public abstract class BaseDirectoryTestC
     in.close(); // close again
     dir.close();
   }
+
+  public void testSeekToEndOfFile() throws IOException {
+    try (Directory dir = newDirectory()) {
+      try (IndexOutput out = dir.createOutput("a", IOContext.DEFAULT)) {
+        for (int i = 0; i < 1024; ++i) {
+          out.writeByte((byte) 0);
+        }
+      }
+      try (IndexInput in = dir.openInput("a", IOContext.DEFAULT)) {
+        in.seek(100);
+        assertEquals(100, in.getFilePointer());
+        in.seek(1024);
+        assertEquals(1024, in.getFilePointer());
+      }
+    }
+  }
+
+  public void testSeekBeyondEndOfFile() throws IOException {
+    try (Directory dir = newDirectory()) {
+      try (IndexOutput out = dir.createOutput("a", IOContext.DEFAULT)) {
+        for (int i = 0; i < 1024; ++i) {
+          out.writeByte((byte) 0);
+        }
+      }
+      try (IndexInput in = dir.openInput("a", IOContext.DEFAULT)) {
+        in.seek(100);
+        assertEquals(100, in.getFilePointer());
+        try {
+          in.seek(1025);
+          fail("didn't hit expected exception");
+        } catch (EOFException eofe) {
+          // expected
+        }
+      }
+    }
+  }
 }



Mime
View raw message