Retire/deprecate per-instance MMapDirectory#setUseUnmap (#12066)

lucene/CHANGES.txt

@@ -145,6 +145,11 @@ API Changes
   provide best performance for filtering and sorting.
   (Francisco Fernández Castaño, Adrien Grand)
 
+* GITHUB#12066: Retire/deprecate instance method MMapDirectory#setUseUnmap().
+  Like the new setting for MemorySegments, this feature is enabled by default and
+  can only be disabled globally by passing the following sysprop on Java command line:
+  "-Dorg.apache.lucene.store.MMapDirectory.enableUnmapHack=false" (Uwe Schindler)
+
 New Features
 ---------------------
 * GITHUB#11795: Add ByteWritesTrackingDirectoryWrapper to expose metrics for bytes merged, flushed, and overall

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

@@ -21,12 +21,15 @@ import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.nio.channels.ClosedChannelException; // javadoc @link
 import java.nio.file.Path;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.Locale;
 import java.util.Optional;
 import java.util.concurrent.Future;
 import java.util.function.BiPredicate;
 import java.util.logging.Logger;
 import org.apache.lucene.util.Constants;
+import org.apache.lucene.util.SuppressForbidden;
 
 /**
  * File-based {@link Directory} implementation that uses mmap for reading, and {@link
@@ -44,10 +47,10 @@ import org.apache.lucene.util.Constants;
  * performance of searches on a cold page cache at the expense of slowing down opening an index. See
  * {@link #setPreload(BiPredicate)} for more details.
  *
- * <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>Due to <a href="https://bugs.openjdk.org/browse/JDK-4724038">this bug</a> in OpenJDK,
+ * 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
@@ -56,14 +59,26 @@ import org.apache.lucene.util.Constants;
  * 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 (see {@link #setUseUnmap}),
- * which may fail on non-Oracle/OpenJDK JVMs. It forcefully unmaps the buffer on close by using an
- * undocumented internal cleanup functionality. If {@link #UNMAP_SUPPORTED} is <code>true</code>,
- * the workaround will be automatically enabled (with no guarantees; if you discover any problems,
- * you can disable it).
+ * <p>This class supplies the workaround mentioned in the bug report, which may fail on
+ * non-Oracle/OpenJDK JVMs. It forcefully unmaps the buffer on close by using an undocumented
+ * internal cleanup functionality. If {@link #UNMAP_SUPPORTED} is <code>true</code>, the workaround
+ * will be automatically enabled (with no guarantees; if you discover any problems, you can disable
+ * it by using system property {@link #ENABLE_UNMAP_HACK_SYSPROP}).
+ *
+ * <p>For the hack to work correct, the following requirements need to be fulfilled: The used JVM
+ * must be at least Oracle Java / OpenJDK. In addition, the following permissions need to be granted
+ * to {@code lucene-core.jar} in your <a
+ * href="http://docs.oracle.com/javase/8/docs/technotes/guides/security/PolicyFiles.html">policy
+ * file</a>:
+ *
+ * <ul>
+ *   <li>{@code permission java.lang.reflect.ReflectPermission "suppressAccessChecks";}
+ *   <li>{@code permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";}
+ * </ul>
  *
  * <p>On exactly <b>Java 19</b> this class will use the modern {@code MemorySegment} API which
- * allows to safely unmap.
+ * allows to safely unmap (if you discover any problems with this preview API, you can disable it by
+ * using system property {@link #ENABLE_MEMORY_SEGMENTS_SYSPROP}).
  *
  * <p><b>NOTE:</b> Accessing this class either directly or indirectly from a thread while it's
  * interrupted can close the underlying channel immediately if at the same time the thread is
@@ -81,6 +96,8 @@ import org.apache.lucene.util.Constants;
  */
 public class MMapDirectory extends FSDirectory {
 
+  private static final Logger LOG = Logger.getLogger(MMapDirectory.class.getName());
+
   /**
    * Argument for {@link #setPreload(BiPredicate)} that configures all files to be preloaded upon
    * opening them.
@@ -100,7 +117,6 @@ public class MMapDirectory extends FSDirectory {
   public static final BiPredicate<String, IOContext> BASED_ON_LOAD_IO_CONTEXT =
       (filename, context) -> context.load;
 
-  private boolean useUnmapHack = UNMAP_SUPPORTED;
   private BiPredicate<String, IOContext> preload = NO_FILES;
 
   /**
@@ -114,6 +130,17 @@ public class MMapDirectory extends FSDirectory {
    */
   public static final long DEFAULT_MAX_CHUNK_SIZE;
 
+  /**
+   * This sysprop allows to control the workaround/hack for unmapping the buffers from address space
+   * after closing {@link IndexInput}. By default it is enabled; set to {@code false} to disable the
+   * unmap hack globally. On command line pass {@code
+   * -Dorg.apache.lucene.store.MMapDirectory.enableUnmapHack=false} to disable.
+   *
+   * @lucene.internal
+   */
+  public static final String ENABLE_UNMAP_HACK_SYSPROP =
+      "org.apache.lucene.store.MMapDirectory.enableUnmapHack";
+
   /**
    * This sysprop allows to control if {@code MemorySegment} API should be used on supported Java
    * versions. By default it is enabled; set to {@code false} to use legacy {@code ByteBuffer}
@@ -195,47 +222,29 @@ public class MMapDirectory extends FSDirectory {
   }
 
   /**
-   * 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-Oracle/OpenJDK JVMs. It forcefully unmaps the buffer on close by using an undocumented
-   * internal cleanup functionality.
+   * This method is retired, see deprecation notice!
    *
-   * <p>On exactly Java 19 this class will use the modern {@code MemorySegment} API which allows to
-   * safely unmap. <em>The following warnings no longer apply in that case!</em>
-   *
-   * <p><b>NOTE:</b> Enabling this is completely unsupported by Java and may lead to JVM crashes if
-   * <code>IndexInput</code> is closed while another thread is still accessing it (SIGSEGV).
-   *
-   * <p>To enable the hack, the following requirements need to be fulfilled: The used JVM must be
-   * Oracle Java / OpenJDK 8 <em>(preliminary support for Java 9 EA build 150+ was added with Lucene
-   * 6.4)</em>. In addition, the following permissions need to be granted to {@code lucene-core.jar}
-   * in your <a
-   * href="http://docs.oracle.com/javase/8/docs/technotes/guides/security/PolicyFiles.html">policy
-   * file</a>:
-   *
-   * <ul>
-   *   <li>{@code permission java.lang.reflect.ReflectPermission "suppressAccessChecks";}
-   *   <li>{@code permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";}
-   * </ul>
-   *
-   * @throws IllegalArgumentException if {@link #UNMAP_SUPPORTED} is <code>false</code> and the
-   *     workaround cannot be enabled. The exception message also contains an explanation why the
-   *     hack cannot be enabled (e.g., missing permissions).
+   * @throws UnsupportedOperationException as setting cannot be changed
+   * @deprecated Please use new system property {@link #ENABLE_UNMAP_HACK_SYSPROP} instead
    */
+  @Deprecated(forRemoval = true)
   public void setUseUnmap(final boolean useUnmapHack) {
-    if (useUnmapHack && !UNMAP_SUPPORTED) {
-      throw new IllegalArgumentException(UNMAP_NOT_SUPPORTED_REASON);
+    if (useUnmapHack != UNMAP_SUPPORTED) {
+      throw new UnsupportedOperationException(
+          "It is no longer possible configure unmap hack for directory instances. Please use the global system property: "
+              + ENABLE_UNMAP_HACK_SYSPROP);
     }
-    this.useUnmapHack = useUnmapHack;
   }
 
   /**
    * Returns <code>true</code>, if the unmap workaround is enabled.
    *
    * @see #setUseUnmap
+   * @deprecated use {@link #UNMAP_SUPPORTED}
    */
+  @Deprecated
   public boolean getUseUnmap() {
-    return useUnmapHack;
+    return UNMAP_SUPPORTED;
   }
 
   /**
@@ -289,8 +298,7 @@ public class MMapDirectory extends FSDirectory {
     ensureOpen();
     ensureCanRead(name);
     Path path = directory.resolve(name);
-    return PROVIDER.openInput(
-        path, context, chunkSizePower, preload.test(name, context), useUnmapHack);
+    return PROVIDER.openInput(path, context, chunkSizePower, preload.test(name, context));
   }
 
   // visible for tests:
@@ -306,8 +314,7 @@ public class MMapDirectory extends FSDirectory {
   public static final String UNMAP_NOT_SUPPORTED_REASON;
 
   static interface MMapIndexInputProvider {
-    IndexInput openInput(
-        Path path, IOContext context, int chunkSizePower, boolean preload, boolean useUnmapHack)
+    IndexInput openInput(Path path, IOContext context, int chunkSizePower, boolean preload)
         throws IOException;
 
     long getDefaultMaxChunkSize();
@@ -359,6 +366,13 @@ public class MMapDirectory extends FSDirectory {
     }
   }
 
+  // Extracted to a method to be able to apply the SuppressForbidden annotation
+  @SuppressWarnings("removal")
+  @SuppressForbidden(reason = "security manager")
+  private static <T> T doPrivileged(PrivilegedAction<T> action) {
+    return AccessController.doPrivileged(action);
+  }
+
   private static boolean checkMemorySegmentsSysprop() {
     try {
       return Optional.ofNullable(System.getProperty(ENABLE_MEMORY_SEGMENTS_SYSPROP))
@@ -367,6 +381,10 @@ public class MMapDirectory extends FSDirectory {
     } catch (
         @SuppressWarnings("unused")
         SecurityException ignored) {
+      LOG.warning(
+          "Cannot read sysprop "
+              + ENABLE_MEMORY_SEGMENTS_SYSPROP
+              + ", so MemorySegments will be enabled by default, if possible.");
       return true;
     }
   }
@@ -398,15 +416,14 @@ public class MMapDirectory extends FSDirectory {
             "MemorySegmentIndexInputProvider is missing in Lucene JAR file", cnfe);
       }
     } else if (runtimeVersion >= 20) {
-      var log = Logger.getLogger(lookup.lookupClass().getName());
-      log.warning(
+      LOG.warning(
           "You are running with Java 20 or later. To make full use of MMapDirectory, please update Apache Lucene.");
     }
     return new MappedByteBufferIndexInputProvider();
   }
 
   static {
-    PROVIDER = lookupProvider();
+    PROVIDER = doPrivileged(MMapDirectory::lookupProvider);
     DEFAULT_MAX_CHUNK_SIZE = PROVIDER.getDefaultMaxChunkSize();
     UNMAP_SUPPORTED = PROVIDER.isUnmapSupported();
     UNMAP_NOT_SUPPORTED_REASON = PROVIDER.getUnmapNotSupportedReason();

lucene/core/src/java/org/apache/lucene/store/MappedByteBufferIndexInputProvider.java

@@ -30,9 +30,8 @@ import java.nio.channels.FileChannel;
 import java.nio.channels.FileChannel.MapMode;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.logging.Logger;
 import org.apache.lucene.store.ByteBufferGuard.BufferCleaner;
 import org.apache.lucene.util.Constants;
@@ -40,13 +39,16 @@ import org.apache.lucene.util.SuppressForbidden;
 
 final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapIndexInputProvider {
 
+  private static final Logger LOG =
+      Logger.getLogger(MappedByteBufferIndexInputProvider.class.getName());
+
   private final BufferCleaner cleaner;
 
   private final boolean unmapSupported;
   private final String unmapNotSupportedReason;
 
   public MappedByteBufferIndexInputProvider() {
-    final Object hack = doPrivileged(MappedByteBufferIndexInputProvider::unmapHackImpl);
+    final Object hack = unmapHackImpl();
     if (hack instanceof BufferCleaner) {
       cleaner = (BufferCleaner) hack;
       unmapSupported = true;
@@ -55,13 +57,12 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
       cleaner = null;
       unmapSupported = false;
       unmapNotSupportedReason = hack.toString();
-      Logger.getLogger(getClass().getName()).warning(unmapNotSupportedReason);
+      LOG.warning(unmapNotSupportedReason);
     }
   }
 
   @Override
-  public IndexInput openInput(
-      Path path, IOContext context, int chunkSizePower, boolean preload, boolean useUnmapHack)
+  public IndexInput openInput(Path path, IOContext context, int chunkSizePower, boolean preload)
       throws IOException {
     if (chunkSizePower > 30) {
       throw new IllegalArgumentException(
@@ -77,7 +78,7 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
           map(resourceDescription, fc, chunkSizePower, preload, fileSize),
           fileSize,
           chunkSizePower,
-          new ByteBufferGuard(resourceDescription, useUnmapHack ? cleaner : null));
+          new ByteBufferGuard(resourceDescription, cleaner));
     }
   }
 
@@ -132,15 +133,29 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
     return buffers;
   }
 
-  // Extracted to a method to be able to apply the SuppressForbidden annotation
-  @SuppressWarnings("removal")
-  @SuppressForbidden(reason = "security manager")
-  private static <T> T doPrivileged(PrivilegedAction<T> action) {
-    return AccessController.doPrivileged(action);
+  private static boolean checkUnmapHackSysprop() {
+    try {
+      return Optional.ofNullable(System.getProperty(MMapDirectory.ENABLE_UNMAP_HACK_SYSPROP))
+          .map(Boolean::valueOf)
+          .orElse(Boolean.TRUE);
+    } catch (
+        @SuppressWarnings("unused")
+        SecurityException ignored) {
+      LOG.warning(
+          "Cannot read sysprop "
+              + MMapDirectory.ENABLE_UNMAP_HACK_SYSPROP
+              + ", so buffer unmap hack will be enabled by default, if possible.");
+      return true;
+    }
   }
 
   @SuppressForbidden(reason = "Needs access to sun.misc.Unsafe to enable hack")
   private static Object unmapHackImpl() {
+    if (checkUnmapHackSysprop() == false) {
+      return "Unmapping was disabled by system property "
+          + MMapDirectory.ENABLE_UNMAP_HACK_SYSPROP
+          + "=false";
+    }
     final Lookup lookup = lookup();
     try {
       // *** sun.misc.Unsafe unmapping (Java 9+) ***
@@ -180,18 +195,10 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
       if (!buffer.isDirect()) {
         throw new IllegalArgumentException("unmapping only works with direct buffers");
       }
-      final Throwable error =
-          doPrivileged(
-              () -> {
-                try {
-                  unmapper.invokeExact(buffer);
-                  return null;
-                } catch (Throwable t) {
-                  return t;
-                }
-              });
-      if (error != null) {
-        throw new IOException("Unable to unmap the mapped buffer: " + resourceDescription, error);
+      try {
+        unmapper.invokeExact(buffer);
+      } catch (Throwable t) {
+        throw new IOException("Unable to unmap the mapped buffer: " + resourceDescription, t);
       }
     };
   }

lucene/core/src/java19/org/apache/lucene/store/MemorySegmentIndexInputProvider.java

@@ -31,12 +31,15 @@ import org.apache.lucene.util.Unwrappable;
 final class MemorySegmentIndexInputProvider implements MMapDirectory.MMapIndexInputProvider {
 
   public MemorySegmentIndexInputProvider() {
-    Logger.getLogger(getClass().getName()).info("Using MemorySegmentIndexInput with Java 19");
+    var log = Logger.getLogger(getClass().getName());
+    log.info(
+        "Using MemorySegmentIndexInput with Java 19; to disable start with -D"
+            + MMapDirectory.ENABLE_MEMORY_SEGMENTS_SYSPROP
+            + "=false");
   }
 
   @Override
-  public IndexInput openInput(
-      Path path, IOContext context, int chunkSizePower, boolean preload, boolean useUnmapHack)
+  public IndexInput openInput(Path path, IOContext context, int chunkSizePower, boolean preload)
       throws IOException {
     final String resourceDescription = "MemorySegmentIndexInput(path=\"" + path.toString() + "\")";
 

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

@@ -65,7 +65,7 @@ public class TestDirectory extends LuceneTestCase {
         assertEquals(1 + largeBuffer.length, d2.fileLength(fname));
 
         // don't do read tests if unmapping is not supported!
-        if (d2 instanceof MMapDirectory && !((MMapDirectory) d2).getUseUnmap()) continue;
+        if (d2 instanceof MMapDirectory && !MMapDirectory.UNMAP_SUPPORTED) continue;
 
         IndexInput input = d2.openInput(fname, newIOContext(random()));
         assertEquals((byte) i, input.readByte());

lucene/test-framework/src/java/org/apache/lucene/tests/store/BaseChunkedDirectoryTestCase.java

@@ -28,7 +28,6 @@ import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.IndexOutput;
-import org.apache.lucene.store.MMapDirectory;
 import org.apache.lucene.tests.analysis.MockAnalyzer;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.tests.util.TestUtil;
@@ -267,10 +266,6 @@ public abstract class BaseChunkedDirectoryTestCase extends BaseDirectoryTestCase
   private void assertChunking(Random random, int chunkSize) throws Exception {
     Path path = createTempDir("mmap" + chunkSize);
     Directory chunkedDir = getDirectory(path, chunkSize);
-    // we will map a lot, try to turn on the unmap hack
-    if (chunkedDir instanceof MMapDirectory && MMapDirectory.UNMAP_SUPPORTED) {
-      ((MMapDirectory) chunkedDir).setUseUnmap(true);
-    }
     MockDirectoryWrapper dir = new MockDirectoryWrapper(random, chunkedDir);
     RandomIndexWriter writer =
         new RandomIndexWriter(