LUCENE-10370: pass proper classpath/module arguments for forking jvms…

… from within tests. (#909)

gradle/java/modules.gradle

@@ -15,6 +15,9 @@
  * limitations under the License.
  */
 
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
 import java.nio.file.Path
 
 // Configure miscellaneous aspects required for supporting the java module system layer.
@@ -172,6 +175,8 @@ allprojects {
     // have the need for it.
     Closure<Void> configureTestTaskForSourceSet = { Test task, SourceSet sourceSet ->
       task.configure {
+        def forkProperties = file("${task.temporaryDir}/jvm-forking.properties")
+
         ModularPathsExtension modularPaths = sourceSet.modularPaths
 
         dependsOn modularPaths
@@ -185,6 +190,30 @@ allprojects {
         doFirst {
           modularPaths.logRuntimePaths(logger)
         }
+
+        // Pass all the required properties for tests which fork the JVM. We don't use
+        // regular system properties here because this could affect task up-to-date checks.
+        jvmArgumentProviders.add(new CommandLineArgumentProvider() {
+          @Override
+          Iterable<String> asArguments() {
+            return ["-Dtests.jvmForkArgsFile=" + forkProperties.absolutePath]
+          }
+        })
+        doFirst {
+          List<String> args = new ArrayList<>(modularPaths.runtimeArguments.asArguments().collect())
+          def cp = modularPaths.runtimeClasspath.asPath
+          if (!cp.isBlank()) {
+            args.addAll(["-cp", cp])
+          }
+
+          // Sanity check.
+          args.forEach(s -> {
+            if (s.contains("\n")) {
+              throw new RuntimeException("LF in forked jvm property?: " + s)
+            }
+          })
+          Files.writeString(forkProperties.toPath(), String.join("\n", args), StandardCharsets.UTF_8)
+        }
       }
     }
 

lucene/CHANGES.txt

@@ -88,7 +88,9 @@ Bug Fixes
 
 Other
 ---------------------
-(No changes)
+
+* LUCENE-10370: pass proper classpath/module arguments for forking jvms from within tests. (Dawid Weiss)
+
 
 ======================= Lucene 9.2.0 =======================
 

lucene/core/src/test/org/apache/lucene/codecs/TestCodecLoadingDeadlock.java

@@ -21,6 +21,7 @@
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Locale;
 import java.util.Random;
 import java.util.Set;
@@ -31,6 +32,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
+import org.apache.lucene.tests.util.LuceneTestCase;
 import org.apache.lucene.util.NamedThreadFactory;
 import org.junit.Assert;
 import org.junit.Test;
@@ -60,18 +62,13 @@ public void testDeadlock() throws Exception {
     System.out.println(
         String.format(Locale.ROOT, "codec: %s, pf: %s, dvf: %s", codecName, pfName, dvfName));
 
+    List<String> args = new ArrayList<>();
+    args.add(Paths.get(System.getProperty("java.home"), "bin", "java").toString());
+    args.addAll(LuceneTestCase.getJvmForkArguments());
+    args.addAll(List.of(getClass().getName(), codecName, pfName, dvfName));
+
     // Fork a separate JVM to reinitialize classes.
-    final Process p =
-        new ProcessBuilder(
-                Paths.get(System.getProperty("java.home"), "bin", "java").toString(),
-                "-cp",
-                System.getProperty("java.class.path"),
-                getClass().getName(),
-                codecName,
-                pfName,
-                dvfName)
-            .inheritIO()
-            .start();
+    final Process p = new ProcessBuilder(args).inheritIO().start();
     if (p.waitFor(MAX_TIME_SECONDS * 2, TimeUnit.SECONDS)) {
       assertEquals("Process died abnormally?", 0, p.waitFor());
     } else {

lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOnJRECrash.java

@@ -104,8 +104,7 @@ public void forkTest() throws Exception {
     cmd.add("-DtempDir=" + tempDir);
     cmd.add("-Dtests.seed=" + SeedUtils.formatSeed(random().nextLong()));
     cmd.add("-ea");
-    cmd.add("-cp");
-    cmd.add(System.getProperty("java.class.path"));
+    cmd.addAll(getJvmForkArguments());
     cmd.add("org.junit.runner.JUnitCore");
     cmd.add(getClass().getName());
     ProcessBuilder pb =

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

@@ -57,24 +57,22 @@ private void runImpl(Class<? extends LockFactory> impl) throws Exception {
           // spawn clients as separate Java processes
           for (int i = 0; i < clients; i++) {
             try {
-              processes.add(
-                  applyRedirection(
-                          new ProcessBuilder(
-                              Paths.get(System.getProperty("java.home"), "bin", "java").toString(),
-                              "-Xmx32M",
-                              "-cp",
-                              System.getProperty("java.class.path"),
-                              LockStressTest.class.getName(),
-                              Integer.toString(i),
-                              addr.getHostString(),
-                              Integer.toString(addr.getPort()),
-                              impl.getName(),
-                              dir.toString(),
-                              Integer.toString(delay),
-                              Integer.toString(rounds)),
-                          i,
-                          dir)
-                      .start());
+              List<String> args = new ArrayList<>();
+              args.add(Paths.get(System.getProperty("java.home"), "bin", "java").toString());
+              args.addAll(getJvmForkArguments());
+              args.addAll(
+                  List.of(
+                      "-Xmx32M",
+                      LockStressTest.class.getName(),
+                      Integer.toString(i),
+                      addr.getHostString(),
+                      Integer.toString(addr.getPort()),
+                      impl.getName(),
+                      dir.toString(),
+                      Integer.toString(delay),
+                      Integer.toString(rounds)));
+
+              processes.add(applyRedirection(new ProcessBuilder(args), i, dir).start());
             } catch (IOException ioe) {
               throw new AssertionError("Failed to start child process.", ioe);
             }

lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestNRTReplication.java

@@ -35,7 +35,6 @@
 import org.apache.lucene.store.LockObtainFailedException;
 import org.apache.lucene.tests.util.LineFileDocs;
 import org.apache.lucene.tests.util.LuceneTestCase;
-import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressCodecs;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressSysoutChecks;
 import org.apache.lucene.tests.util.TestRuleIgnoreTestSuites;
@@ -44,7 +43,6 @@
 import org.apache.lucene.util.SuppressForbidden;
 
 // MockRandom's .sd file has no index header/footer:
-@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/LUCENE-10370")
 @SuppressCodecs({"MockRandom", "Direct", "SimpleText"})
 @SuppressSysoutChecks(bugUrl = "Stuff gets printed, important stuff for debugging a failure")
 public class TestNRTReplication extends LuceneTestCase {
@@ -105,8 +103,9 @@ private NodeProcess startNode(
     long seed = random().nextLong() * nodeStartCounter.incrementAndGet();
     cmd.add("-Dtests.seed=" + SeedUtils.formatSeed(seed));
     cmd.add("-ea");
-    cmd.add("-cp");
-    cmd.add(System.getProperty("java.class.path"));
+
+    cmd.addAll(getJvmForkArguments());
+
     cmd.add("org.junit.runner.JUnitCore");
     cmd.add(TestSimpleServer.class.getName());
 
@@ -355,7 +354,6 @@ public void testReplicateForceMerge() throws Exception {
   // Start up, index 10 docs, replicate, but crash and restart the replica without committing it:
   @Nightly
   public void testReplicaCrashNoCommit() throws Exception {
-
     Path primaryPath = createTempDir("primary");
     NodeProcess primary = startNode(-1, 0, primaryPath, -1, false);
 

lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestStressNRTReplication.java

@@ -27,6 +27,7 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -47,7 +48,6 @@
 import org.apache.lucene.tests.store.MockDirectoryWrapper;
 import org.apache.lucene.tests.util.LineFileDocs;
 import org.apache.lucene.tests.util.LuceneTestCase;
-import org.apache.lucene.tests.util.LuceneTestCase.AwaitsFix;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressCodecs;
 import org.apache.lucene.tests.util.LuceneTestCase.SuppressSysoutChecks;
 import org.apache.lucene.tests.util.TestRuleIgnoreTestSuites;
@@ -115,7 +115,6 @@
  *
  * <p>Slow network is simulated with a RateLimiter.
  */
-@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/LUCENE-10370")
 // MockRandom's .sd file has no index header/footer:
 @SuppressCodecs({"MockRandom", "Direct", "SimpleText"})
 @SuppressSysoutChecks(bugUrl = "Stuff gets printed, important stuff for debugging a failure")
@@ -314,14 +313,9 @@ public void test() throws Exception {
       }
 
       message(
-          "PG="
-              + (primary == null ? "X" : primaryGen)
-              + " "
-              + liveCount
-              + " (of "
-              + nodes.length
-              + ") nodes running: "
-              + sb);
+          ("PG=" + (primary == null ? "X" : primaryGen))
+              + (" " + liveCount)
+              + (" (of " + nodes.length + ") nodes running: " + sb));
 
       // Commit a random node, primary or replica
 
@@ -560,12 +554,7 @@ NodeProcess startNode(final int id, Path indexPath, boolean isPrimary, long forc
 
     NodeProcess curPrimary = primary;
 
-    cmd.add(
-        System.getProperty("java.home")
-            + System.getProperty("file.separator")
-            + "bin"
-            + System.getProperty("file.separator")
-            + "java");
+    cmd.add(Paths.get(System.getProperty("java.home"), "bin", "java").toString());
     cmd.add("-Xmx512m");
 
     if (curPrimary != null) {
@@ -618,8 +607,7 @@ NodeProcess startNode(final int id, Path indexPath, boolean isPrimary, long forc
     long seed = random().nextLong() * nodeStartCounter.incrementAndGet();
     cmd.add("-Dtests.seed=" + SeedUtils.formatSeed(seed));
     cmd.add("-ea");
-    cmd.add("-cp");
-    cmd.add(System.getProperty("java.class.path"));
+    cmd.addAll(getJvmForkArguments());
     cmd.add("org.junit.runner.JUnitCore");
     cmd.add(TestSimpleServer.class.getName());
 

lucene/test-framework/src/java/org/apache/lucene/tests/util/LuceneTestCase.java

@@ -63,6 +63,7 @@
 import java.lang.reflect.Method;
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -106,8 +107,46 @@
 import org.apache.lucene.document.FieldType;
 import org.apache.lucene.document.StringField;
 import org.apache.lucene.document.TextField;
-import org.apache.lucene.index.*;
+import org.apache.lucene.index.BinaryDocValues;
+import org.apache.lucene.index.CodecReader;
+import org.apache.lucene.index.CompositeReader;
+import org.apache.lucene.index.ConcurrentMergeScheduler;
+import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.DocValuesType;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.FieldInfos;
+import org.apache.lucene.index.Fields;
+import org.apache.lucene.index.IndexFileNames;
+import org.apache.lucene.index.IndexOptions;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.LeafReader;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.index.LiveIndexWriterConfig;
+import org.apache.lucene.index.LogByteSizeMergePolicy;
+import org.apache.lucene.index.LogDocMergePolicy;
+import org.apache.lucene.index.LogMergePolicy;
+import org.apache.lucene.index.MergePolicy;
+import org.apache.lucene.index.MergeScheduler;
+import org.apache.lucene.index.MultiBits;
+import org.apache.lucene.index.MultiDocValues;
+import org.apache.lucene.index.MultiTerms;
+import org.apache.lucene.index.NumericDocValues;
+import org.apache.lucene.index.ParallelCompositeReader;
+import org.apache.lucene.index.ParallelLeafReader;
+import org.apache.lucene.index.PointValues;
+import org.apache.lucene.index.PostingsEnum;
+import org.apache.lucene.index.SerialMergeScheduler;
+import org.apache.lucene.index.SimpleMergedSegmentWarmer;
+import org.apache.lucene.index.SortedDocValues;
+import org.apache.lucene.index.SortedNumericDocValues;
+import org.apache.lucene.index.SortedSetDocValues;
+import org.apache.lucene.index.Terms;
+import org.apache.lucene.index.TermsEnum;
 import org.apache.lucene.index.TermsEnum.SeekStatus;
+import org.apache.lucene.index.TieredMergePolicy;
 import org.apache.lucene.internal.tests.IndexPackageAccess;
 import org.apache.lucene.internal.tests.TestSecrets;
 import org.apache.lucene.search.DocIdSetIterator;
@@ -910,7 +949,7 @@ public static void dumpIterator(String label, Iterator<?> iter, PrintStream stre
   /**
    * Convenience method for logging an array. Wraps the array in an iterator and delegates
    *
-   * @see #dumpIterator(String,Iterator,PrintStream)
+   * @see #dumpIterator(String, Iterator, PrintStream)
    */
   public static void dumpArray(String label, Object[] objs, PrintStream stream) {
     Iterator<?> iter = (null == objs) ? null : Arrays.asList(objs).iterator();
@@ -3011,6 +3050,25 @@ public static Path createTempFile() throws IOException {
     return createTempFile("tempFile", ".tmp");
   }
 
+  /**
+   * Returns a set of JVM arguments to fork a JVM with the same class or module path (including any
+   * associated JVM options). The returned value may be empty. This method may throw an assertion
+   * error if fork options cannot be reliably acquired (at the moment they are collected and passed
+   * as an external file in gradle scripts).
+   *
+   * <p><b>JVM forking is strongly discouraged as it makes test slower and more resource-hungry.
+   * Consider all alternatives first.</b>
+   */
+  public static List<String> getJvmForkArguments() throws IOException {
+    String forkArgsFile = System.getProperty("tests.jvmForkArgsFile");
+    Path forkArgsPath;
+    if (forkArgsFile == null || !Files.isRegularFile(forkArgsPath = Paths.get(forkArgsFile))) {
+      throw new AssertionError("JVM fork arguments are not present.");
+    }
+
+    return Files.readAllLines(forkArgsPath, StandardCharsets.UTF_8);
+  }
+
   /**
    * Runs a code part with restricted permissions (be sure to add all required permissions, because
    * it would start with empty permissions). You cannot grant more permissions than our policy file