LUCENE-1658: Fix MMapDirectory, add hack for JVM bug that keeps mmapped files open, fix tests, that cannot use other dir impls than SimpleFSDirectory, some API fine tuning.

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

CHANGES.txt

@@ -146,9 +146,9 @@ API Changes
     is deprecated in favor of the new TimeLimitingCollector which
     extends Collector.  (Shai Erera via Mike McCandless)
 
-13. LUCENE-1621: MultiTermQuery#getTerm() has been deprecated as it does
+13. LUCENE-1621: MultiTermQuery.getTerm() has been deprecated as it does
     not make sense for all subclasses of MultiTermQuery. Check individual
-    subclasses to see if they support #getTerm().  (Mark Miller)
+    subclasses to see if they support getTerm().  (Mark Miller)
 
 14. LUCENE-1636: Make TokenFilter.input final so it's set only
     once. (Wouter Heijke, Uwe Schindler via Mike McCandless).
@@ -156,7 +156,7 @@ API Changes
 15. LUCENE-1658: Renamed FSDirectory to SimpleFSDirectory (but left an
     FSDirectory base class).  Added an FSDirectory.open static method
     to pick a good default FSDirectory implementation given the OS.
-    (Michael McCandless)
+    (Michael McCandless, Uwe Schindler)
 
 Bug fixes
 
@@ -212,6 +212,11 @@ Bug fixes
     rely on this behavior by the 3.0 release of Lucene. (Jonathan
     Mamou, Mark Miller via Mike McCandless)
 
+15. LUCENE-1658: Fixed MMapDirectory to correctly throw IOExceptions
+    on EOF, removed numeric overflow possibilities and added support
+    for a hack to unmap the buffers on closing IndexInput.
+    (Uwe Schindler)
+
  New features
 
  1. LUCENE-1411: Added expert API to open an IndexWriter on a prior

common-build.xml

@@ -42,7 +42,7 @@
   <property name="Name" value="Lucene"/>
   <property name="dev.version" value="2.9-dev"/>
   <property name="version" value="${dev.version}"/>
-  <property name="compatibility.tag" value="lucene_2_4_back_compat_tests_20090530a"/>
+  <property name="compatibility.tag" value="lucene_2_4_back_compat_tests_20090601"/>
   <property name="spec.version" value="${version}"/>	
   <property name="year" value="2000-${current.year}"/>
   <property name="final.name" value="lucene-${name}-${version}"/>

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

@@ -59,12 +59,29 @@ import org.apache.lucene.index.IndexWriter;
  *       choice.
  *
  *  <li> {@link MMapDirectory} uses memory-mapped IO when
- *       reading.  This is a good choice if you have plenty
+ *       reading. This is a good choice if you have plenty
  *       of virtual memory relative to your index size, eg
  *       if you are running on a 64 bit JRE, or you are
  *       running on a 32 bit JRE but your index sizes are
  *       small enough to fit into the virtual memory space.
- *
+ *       Java has currently the limitation of not being able to
+ *       unmap files from user code. The files are unmapped, when GC
+ *       releases the byte buffers. Due to
+ *       <a href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4724038">
+ *       this bug</a> in Sun's JRE, MMapDirectory's {@link IndexInput#close}
+ *       is unable to close the underlying OS file handle. Only when
+ *       GC finally collects the underlying objects, which could be
+ *       quite some time later, will the file handle be closed.
+ *       This will consume additional transient disk usage: on Windows,
+ *       attempts to delete or overwrite the files will result in an
+ *       exception; on other platforms, which typically have a &quot;delete on
+ *       last close&quot; semantics, while such operations will succeed, the bytes
+ *       are still consuming space on disk.  For many applications this
+ *       limitation is not a problem (e.g. if you have plenty of disk space,
+ *       and you don't rely on overwriting files on Windows) but it's still
+ *       an important limitation to be aware of. This class supplies a
+ *       (possibly dangerous) workaround mentioned in the bug report,
+ *       which may fail on non-Sun JVMs.
  * </ul>
  *
  * Unfortunately, because of system peculiarities, there is
@@ -85,6 +102,8 @@ import org.apache.lucene.index.IndexWriter;
  * Java system property, or by calling {@link
  * #setLockFactory} after creating the Directory.
  *
+ * <p><em>In 3.0 this class will become abstract.</em>
+ *
  * @see Directory
  */
 // TODO: in 3.0 this will become an abstract base class
@@ -118,7 +137,8 @@ public class FSDirectory extends Directory {
   /**
    * Returns whether Lucene's use of lock files is disabled.
    * @return true if locks are disabled, false if locks are enabled.
-   */
+   * @see #setDisableLocks
+  */
   public static boolean getDisableLocks() {
     return FSDirectory.disableLocks;
   }
@@ -147,16 +167,17 @@ public class FSDirectory extends Directory {
     try {
       String name =
         System.getProperty("org.apache.lucene.FSDirectory.class",
-                           FSDirectory.class.getName());
+                           SimpleFSDirectory.class.getName());
-      IMPL = Class.forName(name);
+      if (FSDirectory.class.getName().equals(name)) {
+        // FSDirectory will be abstract, so we replace it by the correct class
+        IMPL = SimpleFSDirectory.class;
+      } else {
+        IMPL = Class.forName(name);
+      }
     } catch (ClassNotFoundException e) {
       throw new RuntimeException("cannot load FSDirectory class: " + e.toString(), e);
     } catch (SecurityException se) {
-      try {
+      IMPL = SimpleFSDirectory.class;
-        IMPL = Class.forName(FSDirectory.class.getName());
-      } catch (ClassNotFoundException e) {
-        throw new RuntimeException("cannot load default FSDirectory class: " + e.toString(), e);
-      }
     }
   }
 
@@ -316,7 +337,9 @@ public class FSDirectory extends Directory {
     }
   }
 
-  final void initOutput(String name) throws IOException {
+  /** Initializes the directory to create a new file with the given name.
+   * This method should be used in {@link #createOutput}. */
+  protected final void initOutput(String name) throws IOException {
     ensureOpen();
     createDir();
     File file = new File(directory, name);
@@ -324,18 +347,21 @@ public class FSDirectory extends Directory {
       throw new IOException("Cannot overwrite: " + file);
   }
 
+  /** The underlying filesystem directory */
   protected File directory = null;
-  private int refCount;
+  
+  /** @deprecated */
+  private int refCount = 0;
 
-  protected FSDirectory() {};                     // permit subclassing
+  /** @deprecated */
+  protected FSDirectory() {}; // permit subclassing
 
-  /** Create a new FSDirectory for the named location.
+  /** Create a new FSDirectory for the named location (ctor for subclasses).
-   * @deprecated Use {@link SimpleFSDirectory#SimpleFSDirectory}.
    * @param path the path of the directory
    * @param lockFactory the lock factory to use, or null for the default.
    * @throws IOException
    */
-  public FSDirectory(File path, LockFactory lockFactory) throws IOException {
+  protected FSDirectory(File path, LockFactory lockFactory) throws IOException {
     path = getCanonicalPath(path);
     init(path, lockFactory);
     refCount = 1;
@@ -344,17 +370,20 @@ public class FSDirectory extends Directory {
   /** Creates an FSDirectory instance, trying to pick the
    *  best implementation given the current environment.
    *
-   *  <p>Currently this returns {@link MMapDirectory} when
+   *  <p>Currently this returns {@link NIOFSDirectory}
-   *  running in a 64 bit JRE, {@link NIOFSDirectory} on
+   *  on non-Windows JREs and {@link SimpleFSDirectory}
-   *  non-Windows 32 bit JRE, and {@link SimpleFSDirectory}
+   *  on Windows.
-   *  on Windows 32 bit JRE.
    *
    * <p><b>NOTE</b>: this method may suddenly change which
    * implementation is returned from release to release, in
    * the event that higher performance defaults become
    * possible; if the precise implementation is important to
    * your application, please instantiate it directly,
-   * instead.
+   * instead. On 64 bit systems, it may also good to
+   * return {@link MMapDirectory}, but this is disabled
+   * because of officially missing unmap support in Java.
+   * For optimal performance you should consider using
+   * this implementation on 64 bit JVMs.
    *
    * <p>See <a href="#subclasses">above</a> */
   public static FSDirectory open(File path) throws IOException {
@@ -364,15 +393,19 @@ public class FSDirectory extends Directory {
   /** Just like {@link #open(File)}, but allows you to
    *  also specify a custom {@link LockFactory}. */
   public static FSDirectory open(File path, LockFactory lockFactory) throws IOException {
-    if (Constants.JRE_IS_64BIT) {
+    /* For testing:
-      return new MMapDirectory(path, lockFactory);
+    MMapDirectory dir=new MMapDirectory(path, lockFactory);
-    } else if (Constants.WINDOWS) {
+    dir.setUseUnmap(true);
+    return dir;
+    */
+    if (Constants.WINDOWS) {
       return new SimpleFSDirectory(path, lockFactory);
     } else {
       return new NIOFSDirectory(path, lockFactory);
     }
   }
 
+  /* will move to ctor, when reflection is removed in 3.0 */
   private void init(File path, LockFactory lockFactory) throws IOException {
 
     // Set up lockFactory with cascaded defaults: if an instance was passed in,
@@ -587,15 +620,11 @@ public class FSDirectory extends Directory {
     }
   }
 
-  /** @deprecated In 3.0 this method will become abstract */
+  /** Creates an IndexOutput for the file with the given name.
+   * <em>In 3.0 this method will become abstract.</em> */
   public IndexOutput createOutput(String name) throws IOException {
-    ensureOpen();
+    initOutput(name);
-    createDir();
+    return new FSIndexOutput(new File(directory, name));
-    File file = new File(directory, name);
-    if (file.exists() && !file.delete())          // delete existing, if any
-      throw new IOException("Cannot overwrite: " + file);
-
-    return new FSIndexOutput(file);
   }
 
   public void sync(String name) throws IOException {
@@ -641,7 +670,8 @@ public class FSDirectory extends Directory {
     return openInput(name, BufferedIndexInput.BUFFER_SIZE);
   }
 
-  /** @deprecated In 3.0 this method will become abstract */
+  /** Creates an IndexInput for the file with the given name.
+   * <em>In 3.0 this method will become abstract.</em> */
   public IndexInput openInput(String name, int bufferSize) throws IOException {
     ensureOpen();
     return new FSIndexInput(new File(directory, name), bufferSize);
@@ -698,144 +728,37 @@ public class FSDirectory extends Directory {
     return this.getClass().getName() + "@" + directory;
   }
 
+
   /** @deprecated Use SimpleFSDirectory.SimpleFSIndexInput instead */
-  protected static class FSIndexInput extends BufferedIndexInput {
+  protected static class FSIndexInput extends SimpleFSDirectory.SimpleFSIndexInput {
   
-    protected static class Descriptor extends RandomAccessFile {
+    /** @deprecated */
-      // remember if the file is open, so that we don't try to close it
+    protected static class Descriptor extends SimpleFSDirectory.SimpleFSIndexInput.Descriptor {
-      // more than once
+      /** @deprecated */
-      protected volatile boolean isOpen;
-      long position;
-      final long length;
-      
       public Descriptor(File file, String mode) throws IOException {
         super(file, mode);
-        isOpen=true;
-        length=length();
-      }
-  
-      public void close() throws IOException {
-        if (isOpen) {
-          isOpen=false;
-          super.close();
-        }
-      }
-  
-      protected void finalize() throws Throwable {
-        try {
-          close();
-        } finally {
-          super.finalize();
-        }
       }
     }
   
-    protected final Descriptor file;
+    /** @deprecated */
-    boolean isClone;
-  
     public FSIndexInput(File path) throws IOException {
-      this(path, BufferedIndexInput.BUFFER_SIZE);
+      super(path);
     }
   
+    /** @deprecated */
     public FSIndexInput(File path, int bufferSize) throws IOException {
-      super(bufferSize);
+      super(path, bufferSize);
-      file = new Descriptor(path, "r");
     }
   
-    /** IndexInput methods */
-    protected void readInternal(byte[] b, int offset, int len)
-         throws IOException {
-      synchronized (file) {
-        long position = getFilePointer();
-        if (position != file.position) {
-          file.seek(position);
-          file.position = position;
-        }
-        int total = 0;
-        do {
-          int i = file.read(b, offset+total, len-total);
-          if (i == -1)
-            throw new IOException("read past EOF");
-          file.position += i;
-          total += i;
-        } while (total < len);
-      }
-    }
-  
-    public void close() throws IOException {
-      // only close the file if this is not a clone
-      if (!isClone) file.close();
-    }
-  
-    protected void seekInternal(long position) {
-    }
-  
-    public long length() {
-      return file.length;
-    }
-  
-    public Object clone() {
-      FSIndexInput clone = (FSIndexInput)super.clone();
-      clone.isClone = true;
-      return clone;
-    }
-  
-    /** Method used for testing. Returns true if the underlying
-     *  file descriptor is valid.
-     */
-    boolean isFDValid() throws IOException {
-      return file.getFD().valid();
-    }
   }
 
   /** @deprecated Use SimpleFSDirectory.SimpleFSIndexOutput instead */
-  protected static class FSIndexOutput extends BufferedIndexOutput {
+  protected static class FSIndexOutput extends SimpleFSDirectory.SimpleFSIndexOutput {
-    RandomAccessFile file = null;
-  
-    // remember if the file is open, so that we don't try to close it
-    // more than once
-    private volatile boolean isOpen;
 
+    /** @deprecated */
     public FSIndexOutput(File path) throws IOException {
-      file = new RandomAccessFile(path, "rw");
+      super(path);
-      isOpen = true;
-    }
-  
-    /** output methods: */
-    public void flushBuffer(byte[] b, int offset, int size) throws IOException {
-      file.write(b, offset, size);
-    }
-    public void close() throws IOException {
-      // only close the file if it has not been closed yet
-      if (isOpen) {
-        boolean success = false;
-        try {
-          super.close();
-          success = true;
-        } finally {
-          isOpen = false;
-          if (!success) {
-            try {
-              file.close();
-            } catch (Throwable t) {
-              // Suppress so we don't mask original exception
-            }
-          } else
-            file.close();
-        }
-      }
-    }
-  
-    /** Random-access methods */
-    public void seek(long pos) throws IOException {
-      super.seek(pos);
-      file.seek(pos);
-    }
-    public long length() throws IOException {
-      return file.length();
-    }
-    public void setLength(long length) throws IOException {
-      file.setLength(length);
     }
+
   }
 }

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

@@ -21,19 +21,47 @@ import java.io.IOException;
 import java.io.File;
 import java.io.RandomAccessFile;
 import java.nio.ByteBuffer;
+import java.nio.BufferUnderflowException;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileChannel.MapMode;
 
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
+import java.lang.reflect.Method;
+
 /** File-based {@link Directory} implementation that uses
  *  mmap for reading, and {@link
  *  SimpleFSDirectory.SimpleFSIndexOutput} for writing.
  *
- * <p> <b>NOTE</b>: memory mapping uses up a portion of the
+ * <p><b>NOTE</b>: memory mapping uses up a portion of the
  * virtual memory address space in your process equal to the
  * size of the file being mapped.  Before using this class,
- * be sure your have plenty of virtual memory, eg by using a
+ * be sure your have plenty of virtual address space, e.g. by
- * 64 bit JRE, or a 32 bit JRE with indexes that are
+ * using a 64 bit JRE, or a 32 bit JRE with indexes that are
  * guaranteed to fit within the address space.
+ *
+ * <p>Due to <a href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4724038">
+ * this bug</a> in Sun's JRE, MMapDirectory's {@link IndexInput#close}
+ * is unable to close the underlying OS file handle.  Only when GC
+ * finally collects the underlying objects, which could be quite
+ * some time later, will the file handle be closed.
+ *
+ * <p>This will consume additional transient disk usage: on Windows,
+ * attempts to delete or overwrite the files will result in an
+ * exception; on other platforms, which typically have a &quot;delete on
+ * last close&quot; semantics, while such operations will succeed, the bytes
+ * are still consuming space on disk.  For many applications this
+ * limitation is not a problem (e.g. if you have plenty of disk space,
+ * and you don't rely on overwriting files on Windows) but it's still
+ * an important limitation to be aware of.
+ *
+ * <p>This class supplies the workaround mentioned in the bug report
+ * (disabled by default, see {@link #setUseUnmap}), which may fail on
+ * non-Sun JVMs. It forcefully unmaps the buffer on close by using
+ * an undocumented internal cleanup functionality.
+ * {@link #UNMAP_SUPPORTED} is <code>true</code>, if the workaround
+ * can be enabled (with no guarantees).
  */
 public class MMapDirectory extends FSDirectory {
 
@@ -47,16 +75,101 @@ public class MMapDirectory extends FSDirectory {
     super(path, lockFactory);
   }
 
-  // back compatibility so FSDirectory can instantiate via
+  /** Create a new MMapDirectory for the named location and the default lock factory.
-  // reflection
+   *
-  /* @deprecated */
+   * @param path the path of the directory
-  protected MMapDirectory() throws IOException {
+   * @throws IOException
+   */
+  public MMapDirectory(File path) throws IOException {
+    super(path, null);
   }
 
-  private static class MMapIndexInput extends IndexInput {
+  // back compatibility so FSDirectory can instantiate via reflection
+  /** @deprecated */
+  MMapDirectory() {}
+  
+  static final Class[] NO_PARAM_TYPES = new Class[0];
+  static final Object[] NO_PARAMS = new Object[0];
+  
+  private boolean useUnmapHack = false;
+  
+  /**
+   * <code>true</code>, if this platform supports unmapping mmaped files.
+   */
+  public static final boolean UNMAP_SUPPORTED;
+  static {
+    boolean v;
+    try {
+      Class.forName("sun.misc.Cleaner");
+      Class.forName("java.nio.DirectByteBuffer")
+        .getMethod("cleaner", NO_PARAM_TYPES);
+      v = true;
+    } catch (Exception e) {
+      v = false;
+    }
+    UNMAP_SUPPORTED = v;
+  }
+
+  /**
+   * This method enables the workaround for unmapping the buffers
+   * from address space after closing {@link IndexInput}, that is
+   * mentioned in the bug report. This hack may fail on non-Sun JVMs.
+   * It forcefully unmaps the buffer on close by using
+   * an undocumented internal cleanup functionality.
+   * <p><b>NOTE:</b> Enabling this is completely unsupported
+   * by Java and may lead to JVM crashs if <code>IndexInput</code>
+   * is closed while another thread is still accessing it (SIGSEGV).
+   * @throws IllegalArgumentException if {@link #UNMAP_SUPPORTED}
+   * is <code>false</code> and the workaround cannot be enabled.
+   */
+  public void setUseUnmap(final boolean useUnmapHack) {
+    if (useUnmapHack && !UNMAP_SUPPORTED)
+      throw new IllegalArgumentException("Unmap hack not supported on this platform!");
+    this.useUnmapHack=useUnmapHack;
+  }
+  
+  /**
+   * Returns <code>true</code>, if the unmap workaround is enabled.
+   * @see #setUseUnmap
+   */
+  public boolean getUseUnmap() {
+    return useUnmapHack;
+  }
+  
+  /**
+   * Try to unmap the buffer, this method silently fails if no support
+   * for that in the JVM. On Windows, this leads to the fact,
+   * that mmapped files cannot be modified or deleted.
+   */
+  final void cleanMapping(final ByteBuffer buffer) throws IOException {
+    if (useUnmapHack) {
+      try {
+        AccessController.doPrivileged(new PrivilegedExceptionAction() {
+          public Object run() throws Exception {
+            final Method getCleanerMethod = buffer.getClass()
+              .getMethod("cleaner", NO_PARAM_TYPES);
+            getCleanerMethod.setAccessible(true);
+            final Object cleaner = getCleanerMethod.invoke(buffer, NO_PARAMS);
+            if (cleaner != null) {
+              cleaner.getClass().getMethod("clean", NO_PARAM_TYPES)
+                .invoke(cleaner, NO_PARAMS);
+            }
+            return null;
+          }
+        });
+      } catch (PrivilegedActionException e) {
+        final IOException ioe = new IOException("unable to unmap the mapped buffer");
+        ioe.initCause(e.getCause());
+        throw ioe;
+      }
+    }
+  }
+
+  private class MMapIndexInput extends IndexInput {
 
     private ByteBuffer buffer;
     private final long length;
+    private boolean isClone = false;
 
     private MMapIndexInput(RandomAccessFile raf) throws IOException {
         this.length = raf.length();
@@ -64,12 +177,19 @@ public class MMapDirectory extends FSDirectory {
     }
 
     public byte readByte() throws IOException {
-      return buffer.get();
+      try {
+        return buffer.get();
+      } catch (BufferUnderflowException e) {
+        throw new IOException("read past EOF");
+      }
     }
 
-    public void readBytes(byte[] b, int offset, int len)
+    public void readBytes(byte[] b, int offset, int len) throws IOException {
-      throws IOException {
+      try {
-      buffer.get(b, offset, len);
+        buffer.get(b, offset, len);
+      } catch (BufferUnderflowException e) {
+        throw new IOException("read past EOF");
+      }
     }
 
     public long getFilePointer() {
@@ -86,17 +206,26 @@ public class MMapDirectory extends FSDirectory {
 
     public Object clone() {
       MMapIndexInput clone = (MMapIndexInput)super.clone();
+      clone.isClone = true;
       clone.buffer = buffer.duplicate();
       return clone;
     }
 
-    public void close() throws IOException {}
+    public void close() throws IOException {
+      if (isClone || buffer == null) return;
+      // unmap the buffer (if enabled) and at least unset it for GC
+      try {
+        cleanMapping(buffer);
+      } finally {
+        buffer = null;
+      }
+    }
   }
 
   // Because Java's ByteBuffer uses an int to address the
   // values, it's necessary to access a file >
   // Integer.MAX_VALUE in size using multiple byte buffers.
-  private static class MultiMMapIndexInput extends IndexInput {
+  private class MultiMMapIndexInput extends IndexInput {
   
     private ByteBuffer[] buffers;
     private int[] bufSizes; // keep here, ByteBuffer.size() method is optional
@@ -109,6 +238,7 @@ public class MMapDirectory extends FSDirectory {
     private ByteBuffer curBuf; // redundant for speed: buffers[curBufIndex]
     private int curAvail; // redundant for speed: (bufSizes[curBufIndex] - curBuf.position())
   
+    private boolean isClone = false;
     
     public MultiMMapIndexInput(RandomAccessFile raf, int maxBufSize)
       throws IOException {
@@ -125,7 +255,7 @@ public class MMapDirectory extends FSDirectory {
            + raf.toString());
       
       int nrBuffers = (int) (length / maxBufSize);
-      if ((nrBuffers * maxBufSize) < length) nrBuffers++;
+      if (((long) nrBuffers * maxBufSize) < length) nrBuffers++;
       
       this.buffers = new ByteBuffer[nrBuffers];
       this.bufSizes = new int[nrBuffers];
@@ -145,10 +275,12 @@ public class MMapDirectory extends FSDirectory {
   
     public byte readByte() throws IOException {
       // Performance might be improved by reading ahead into an array of
-      // eg. 128 bytes and readByte() from there.
+      // e.g. 128 bytes and readByte() from there.
       if (curAvail == 0) {
         curBufIndex++;
-        curBuf = buffers[curBufIndex]; // index out of bounds when too many bytes requested
+        if (curBufIndex >= buffers.length)
+          throw new IOException("read past EOF");
+        curBuf = buffers[curBufIndex];
         curBuf.position(0);
         curAvail = bufSizes[curBufIndex];
       }
@@ -162,7 +294,9 @@ public class MMapDirectory extends FSDirectory {
         len -= curAvail;
         offset += curAvail;
         curBufIndex++;
-        curBuf = buffers[curBufIndex]; // index out of bounds when too many bytes requested
+        if (curBufIndex >= buffers.length)
+          throw new IOException("read past EOF");
+        curBuf = buffers[curBufIndex];
         curBuf.position(0);
         curAvail = bufSizes[curBufIndex];
       }
@@ -171,13 +305,13 @@ public class MMapDirectory extends FSDirectory {
     }
   
     public long getFilePointer() {
-      return (curBufIndex * (long) maxBufSize) + curBuf.position();
+      return ((long) curBufIndex * maxBufSize) + curBuf.position();
     }
   
     public void seek(long pos) throws IOException {
       curBufIndex = (int) (pos / maxBufSize);
       curBuf = buffers[curBufIndex];
-      int bufOffset = (int) (pos - (curBufIndex * maxBufSize));
+      int bufOffset = (int) (pos - ((long) curBufIndex * maxBufSize));
       curBuf.position(bufOffset);
       curAvail = bufSizes[curBufIndex] - bufOffset;
     }
@@ -188,10 +322,11 @@ public class MMapDirectory extends FSDirectory {
   
     public Object clone() {
       MultiMMapIndexInput clone = (MultiMMapIndexInput)super.clone();
+      clone.isClone = true;
       clone.buffers = new ByteBuffer[buffers.length];
       // No need to clone bufSizes.
       // Since most clones will use only one buffer, duplicate() could also be
-      // done lazy in clones, eg. when adapting curBuf.
+      // done lazy in clones, e.g. when adapting curBuf.
       for (int bufNr = 0; bufNr < buffers.length; bufNr++) {
         clone.buffers[bufNr] = buffers[bufNr].duplicate();
       }
@@ -205,15 +340,26 @@ public class MMapDirectory extends FSDirectory {
       return clone;
     }
   
-    public void close() throws IOException {}
+    public void close() throws IOException {
+      if (isClone || buffers == null) return;
+      try {
+        for (int bufNr = 0; bufNr < buffers.length; bufNr++) {
+          // unmap the buffer (if enabled) and at least unset it for GC
+          try {
+            cleanMapping(buffers[bufNr]);
+          } finally {
+            buffers[bufNr] = null;
+          }
+        }
+      } finally {
+        buffers = null;
+      }
+    }
   }
   
   private final int MAX_BBUF = Integer.MAX_VALUE;
 
-  public IndexInput openInput(String name) throws IOException {
+  /** Creates an IndexInput for the file with the given name. */
-    return openInput(name, BufferedIndexInput.BUFFER_SIZE);
-  }
-
   public IndexInput openInput(String name, int bufferSize) throws IOException {
     ensureOpen();
     File f =  new File(getFile(), name);
@@ -227,6 +373,7 @@ public class MMapDirectory extends FSDirectory {
     }
   }
 
+  /** Creates an IndexOutput for the file with the given name. */
   public IndexOutput createOutput(String name) throws IOException {
     initOutput(name);
     return new SimpleFSDirectory.SimpleFSIndexOutput(new File(directory, name));

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

@@ -50,23 +50,32 @@ public class NIOFSDirectory extends FSDirectory {
     super(path, lockFactory);
   }
 
-  // back compatibility so FSDirectory can instantiate via reflection
+  /** Create a new NIOFSDirectory for the named location and the default lock factory.
-  /* @deprecated */
+   *
-  protected NIOFSDirectory() throws IOException {
+   * @param path the path of the directory
+   * @throws IOException
+   */
+  public NIOFSDirectory(File path) throws IOException {
+    super(path, null);
   }
 
-  // Inherit javadoc
+  // back compatibility so FSDirectory can instantiate via reflection
+  /** @deprecated */
+  NIOFSDirectory() {}
+
+  /** Creates an IndexInput for the file with the given name. */
   public IndexInput openInput(String name, int bufferSize) throws IOException {
     ensureOpen();
     return new NIOFSIndexInput(new File(getFile(), name), bufferSize);
   }
 
+  /** Creates an IndexOutput for the file with the given name. */
   public IndexOutput createOutput(String name) throws IOException {
     initOutput(name);
     return new SimpleFSDirectory.SimpleFSIndexOutput(new File(directory, name));
   }
 
-  private static class NIOFSIndexInput extends FSDirectory.FSIndexInput {
+  private static class NIOFSIndexInput extends SimpleFSDirectory.SimpleFSIndexInput {
 
     private ByteBuffer byteBuf; // wraps the buffer for NIO
 

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

@@ -38,19 +38,27 @@ public class SimpleFSDirectory extends FSDirectory {
   public SimpleFSDirectory(File path, LockFactory lockFactory) throws IOException {
     super(path, lockFactory);
   }
-
+  
-  // Inherit javadoc
+  /** Create a new SimpleFSDirectory for the named location and the default lock factory.
-  public IndexOutput createOutput(String name) throws IOException {
+   *
-    ensureOpen();
+   * @param path the path of the directory
-    createDir();
+   * @throws IOException
-    File file = new File(directory, name);
+   */
-    if (file.exists() && !file.delete())          // delete existing, if any
+  public SimpleFSDirectory(File path) throws IOException {
-      throw new IOException("Cannot overwrite: " + file);
+    super(path, null);
-
-    return new SimpleFSIndexOutput(file);
   }
 
-  // Inherit javadoc
+  // back compatibility so FSDirectory can instantiate via reflection
+  /** @deprecated */
+  SimpleFSDirectory() {}
+
+  /** Creates an IndexOutput for the file with the given name. */
+  public IndexOutput createOutput(String name) throws IOException {
+    initOutput(name);
+    return new SimpleFSIndexOutput(new File(directory, name));
+  }
+
+  /** Creates an IndexInput for the file with the given name. */
   public IndexInput openInput(String name, int bufferSize) throws IOException {
     ensureOpen();
     return new SimpleFSIndexInput(new File(directory, name), bufferSize);

src/test/org/apache/lucene/index/TestCompoundFile.java

@@ -26,7 +26,7 @@ import junit.textui.TestRunner;
 import org.apache.lucene.store.IndexOutput;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IndexInput;
-import org.apache.lucene.store.FSDirectory;
+import org.apache.lucene.store.SimpleFSDirectory;
 import org.apache.lucene.store._TestHelper;
 import org.apache.lucene.util._TestUtil;
 
@@ -62,9 +62,14 @@ public class TestCompoundFile extends LuceneTestCase
        super.setUp();
        File file = new File(System.getProperty("tempDir"), "testIndex");
        _TestUtil.rmDir(file);
-       dir = FSDirectory.open(file);
+       // use a simple FSDir here, to be sure to have SimpleFSInputs
+       dir = new SimpleFSDirectory(file,null);
     }
 
+    public void tearDown() throws Exception {
+       dir.close();
+       super.tearDown();
+    }
 
     /** Creates a file of the specified size with random data. */
     private void createRandomFile(Directory dir, String name, int size)
@@ -310,10 +315,10 @@ public class TestCompoundFile extends LuceneTestCase
 
 
     public void testReadAfterClose() throws IOException {
-        demo_FSIndexInputBug((FSDirectory) dir, "test");
+        demo_FSIndexInputBug(dir, "test");
     }
 
-    private void demo_FSIndexInputBug(FSDirectory fsdir, String file)
+    private void demo_FSIndexInputBug(Directory fsdir, String file)
     throws IOException
     {
         // Setup the test file - we need more than 1024 bytes
@@ -358,7 +363,7 @@ public class TestCompoundFile extends LuceneTestCase
             CompoundFileReader.CSIndexInput cis =
             (CompoundFileReader.CSIndexInput) is;
 
-            return _TestHelper.isFSIndexInputOpen(cis.base);
+            return _TestHelper.isSimpleFSIndexInputOpen(cis.base);
         } else {
             return false;
         }
@@ -373,49 +378,47 @@ public class TestCompoundFile extends LuceneTestCase
         IndexInput expected = dir.openInput("f11");
 
         // this test only works for FSIndexInput
-        if (_TestHelper.isFSIndexInput(expected)) {
+        assertTrue(_TestHelper.isSimpleFSIndexInput(expected));
+        assertTrue(_TestHelper.isSimpleFSIndexInputOpen(expected));
 
-          assertTrue(_TestHelper.isFSIndexInputOpen(expected));
+        IndexInput one = cr.openInput("f11");
+        assertTrue(isCSIndexInputOpen(one));
 
-          IndexInput one = cr.openInput("f11");
+        IndexInput two = (IndexInput) one.clone();
-          assertTrue(isCSIndexInputOpen(one));
+        assertTrue(isCSIndexInputOpen(two));
 
-          IndexInput two = (IndexInput) one.clone();
+        assertSameStreams("basic clone one", expected, one);
-          assertTrue(isCSIndexInputOpen(two));
+        expected.seek(0);
+        assertSameStreams("basic clone two", expected, two);
 
-          assertSameStreams("basic clone one", expected, one);
+        // Now close the first stream
-          expected.seek(0);
+        one.close();
-          assertSameStreams("basic clone two", expected, two);
+        assertTrue("Only close when cr is closed", isCSIndexInputOpen(one));
 
-          // Now close the first stream
+        // The following should really fail since we couldn't expect to
-          one.close();
+        // access a file once close has been called on it (regardless of
-          assertTrue("Only close when cr is closed", isCSIndexInputOpen(one));
+        // buffering and/or clone magic)
-
+        expected.seek(0);
-          // The following should really fail since we couldn't expect to
+        two.seek(0);
-          // access a file once close has been called on it (regardless of
+        assertSameStreams("basic clone two/2", expected, two);
-          // buffering and/or clone magic)
-          expected.seek(0);
-          two.seek(0);
-          assertSameStreams("basic clone two/2", expected, two);
 
 
-          // Now close the compound reader
+        // Now close the compound reader
-          cr.close();
+        cr.close();
-          assertFalse("Now closed one", isCSIndexInputOpen(one));
+        assertFalse("Now closed one", isCSIndexInputOpen(one));
-          assertFalse("Now closed two", isCSIndexInputOpen(two));
+        assertFalse("Now closed two", isCSIndexInputOpen(two));
 
-          // The following may also fail since the compound stream is closed
+        // The following may also fail since the compound stream is closed
-          expected.seek(0);
+        expected.seek(0);
-          two.seek(0);
+        two.seek(0);
-          //assertSameStreams("basic clone two/3", expected, two);
+        //assertSameStreams("basic clone two/3", expected, two);
 
 
-          // Now close the second clone
+        // Now close the second clone
-          two.close();
+        two.close();
-          expected.seek(0);
+        expected.seek(0);
-          two.seek(0);
+        two.seek(0);
-          //assertSameStreams("basic clone two/4", expected, two);
+        //assertSameStreams("basic clone two/4", expected, two);
-        }
 
         expected.close();
     }

src/test/org/apache/lucene/index/TestDoc.java

@@ -59,7 +59,7 @@ public class TestDoc extends LuceneTestCase {
         indexDir = new File(workDir, "testIndex");
         indexDir.mkdirs();
 
-        Directory directory = FSDirectory.getDirectory(indexDir, true);
+        Directory directory = FSDirectory.open(indexDir);
         directory.close();
 
         files = new LinkedList();

src/test/org/apache/lucene/index/TestIndexModifier.java

@@ -144,7 +144,7 @@ public class TestIndexModifier extends LuceneTestCase {
     if (tempDir == null)
       throw new IOException("java.io.tmpdir undefined, cannot run test");
     File indexDir = new File(tempDir, "lucenetestindex");
-    Directory rd = FSDirectory.getDirectory(indexDir);
+    Directory rd = FSDirectory.open(indexDir);
     IndexThread.id = 0;
     IndexThread.idStack.clear();
     IndexModifier index = new IndexModifier(rd, new StandardAnalyzer(), create);

src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -4207,7 +4207,7 @@ public class TestIndexWriter extends LuceneTestCase
   
   public void testOtherFiles() throws Throwable {
     File indexDir = new File(System.getProperty("tempDir"), "otherfiles");
-    Directory dir = new FSDirectory(indexDir, null);
+    Directory dir = FSDirectory.open(indexDir);
     try {
       // Create my own random file:
 

src/test/org/apache/lucene/index/TestStressIndexing2.java

@@ -69,7 +69,7 @@ public class TestStressIndexing2 extends LuceneTestCase {
   public void testRandom() throws Throwable {
     r = newRandom();
     Directory dir1 = new MockRAMDirectory();
-    // dir1 = FSDirectory.getDirectory("foofoofoo");
+    // dir1 = FSDirectory.open("foofoofoo");
     Directory dir2 = new MockRAMDirectory();
     // mergeFactor=2; maxBufferedDocs=2; Map docs = indexRandom(1, 3, 2, dir1);
     Map docs = indexRandom(10, 100, 100, dir1);

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

@@ -212,7 +212,7 @@ public class TestBufferedIndexInput extends LuceneTestCase {
       public MockFSDirectory(File path, Random rand) throws IOException {
         this.rand = rand;
         lockFactory = new NoLockFactory();
-        dir = FSDirectory.open(path);
+        dir = new SimpleFSDirectory(path, null);
       }
 
       public IndexInput openInput(String name) throws IOException {

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

@@ -53,7 +53,7 @@ public class TestDirectory extends LuceneTestCase {
     int sz = 3;
     Directory[] dirs = new Directory[sz];
 
-    dirs[0] = new FSDirectory(path, null);
+    dirs[0] = new SimpleFSDirectory(path, null);
     dirs[1] = new NIOFSDirectory(path, null);
     dirs[2] = new MMapDirectory(path, null);
 
@@ -123,7 +123,7 @@ public class TestDirectory extends LuceneTestCase {
     File path = new File(System.getProperty("tempDir"), "doesnotexist");
     try {
       assertTrue(!path.exists());
-      Directory dir = new FSDirectory(path, null);
+      Directory dir = new SimpleFSDirectory(path, null);
       assertTrue(!path.exists());
       dir.close();
     } finally {
@@ -159,7 +159,7 @@ public class TestDirectory extends LuceneTestCase {
     try {
       path.mkdirs();
       new File(path, "subdir").mkdirs();
-      Directory fsDir = new FSDirectory(path, null);
+      Directory fsDir = new SimpleFSDirectory(path, null);
       assertEquals(0, new RAMDirectory(fsDir).listAll().length);
     } finally {
       _TestUtil.rmDir(path);
@@ -169,13 +169,13 @@ public class TestDirectory extends LuceneTestCase {
   // LUCENE-1468
   public void testNotDirectory() throws Throwable {
     File path = new File(System.getProperty("tempDir"), "testnotdir");
-    Directory fsDir = new FSDirectory(path, null);
+    Directory fsDir = new SimpleFSDirectory(path, null);
     try {
       IndexOutput out = fsDir.createOutput("afile");
       out.close();
       assertTrue(fsDir.fileExists("afile"));
       try {
-        new FSDirectory(new File(path, "afile"), null);
+        new SimpleFSDirectory(new File(path, "afile"), null);
         fail("did not hit expected exception");
       } catch (NoSuchDirectoryException nsde) {
         // Expected

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

@@ -1,52 +0,0 @@
-package org.apache.lucene.store;
-
-/**
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import org.apache.lucene.util.LuceneTestCase;
-import java.lang.reflect.Method;
-
-public class TestMMapDirectory extends LuceneTestCase {
-
-  // Simply verify that if there is a method in FSDirectory
-  // that returns IndexInput or a subclass, that
-  // MMapDirectory overrides it.
-  public void testIndexInputMethods() throws ClassNotFoundException  {
-    
-    Class FSDirectory = Class.forName("org.apache.lucene.store.FSDirectory");
-    Class IndexInput = Class.forName("org.apache.lucene.store.IndexInput");
-    Class MMapDirectory = Class.forName("org.apache.lucene.store.MMapDirectory");
-
-    Method[] methods = FSDirectory.getDeclaredMethods();
-    for(int i=0;i<methods.length;i++) {
-      Method method = methods[i];
-      if (IndexInput.isAssignableFrom(method.getReturnType())) {
-        // There is a method that returns IndexInput or a
-        // subclass of IndexInput
-        try {
-          Method m = MMapDirectory.getMethod(method.getName(), method.getParameterTypes());
-          if (m.getDeclaringClass() != MMapDirectory) {
-            fail("FSDirectory has method " + method + " but MMapDirectory does not override");
-          }
-        } catch (NoSuchMethodException e) {
-          // Should not happen
-          fail("unexpected NoSuchMethodException");
-        }
-      }
-    }
-  }
-}

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

@@ -19,7 +19,7 @@ package org.apache.lucene.store;
 
 import java.io.IOException;
 
-import org.apache.lucene.store.FSDirectory.FSIndexInput;
+import org.apache.lucene.store.SimpleFSDirectory.SimpleFSIndexInput;
 
 /** This class provides access to package-level features defined in the
  *  store package. It is used for testing only.
@@ -27,35 +27,35 @@ import org.apache.lucene.store.FSDirectory.FSIndexInput;
 public class _TestHelper {
 
     /** Returns true if the instance of the provided input stream is actually
-     *  an FSIndexInput.
+     *  an SimpleFSIndexInput.
      */
-    public static boolean isFSIndexInput(IndexInput is) {
+    public static boolean isSimpleFSIndexInput(IndexInput is) {
-        return is instanceof FSIndexInput;
+        return is instanceof SimpleFSIndexInput;
     }
 
-    /** Returns true if the provided input stream is an FSIndexInput and
+    /** Returns true if the provided input stream is an SimpleFSIndexInput and
      *  is a clone, that is it does not own its underlying file descriptor.
      */
-    public static boolean isFSIndexInputClone(IndexInput is) {
+    public static boolean isSimpleFSIndexInputClone(IndexInput is) {
-        if (isFSIndexInput(is)) {
+        if (isSimpleFSIndexInput(is)) {
-            return ((FSIndexInput) is).isClone;
+            return ((SimpleFSIndexInput) is).isClone;
         } else {
             return false;
         }
     }
 
-    /** Given an instance of FSDirectory.FSIndexInput, this method returns
+    /** Given an instance of SimpleFSDirectory.SimpleFSIndexInput, this method returns
      *  true if the underlying file descriptor is valid, and false otherwise.
      *  This can be used to determine if the OS file has been closed.
      *  The descriptor becomes invalid when the non-clone instance of the
-     *  FSIndexInput that owns this descriptor is closed. However, the
+     *  SimpleFSIndexInput that owns this descriptor is closed. However, the
      *  descriptor may possibly become invalid in other ways as well.
      */
-    public static boolean isFSIndexInputOpen(IndexInput is)
+    public static boolean isSimpleFSIndexInputOpen(IndexInput is)
     throws IOException
     {
-        if (isFSIndexInput(is)) {
+        if (isSimpleFSIndexInput(is)) {
-            FSIndexInput fis = (FSIndexInput) is;
+            SimpleFSIndexInput fis = (SimpleFSIndexInput) is;
             return fis.isFDValid();
         } else {
             return false;