From eb2cb90177e4e3f71152fd17854c0be47c1fdf67 Mon Sep 17 00:00:00 2001
From: "(skovati) Luke" <skovati@protonmail.com>
Date: Mon, 12 Feb 2024 15:22:07 -0800
Subject: [PATCH] Remove thread lifecycle tests in lieu of loom

---
 .../driver/engine/SimulationEngine.java       | 24 +------------
 .../simulation/ResumableSimulationTest.java   | 36 -------------------
 2 files changed, 1 insertion(+), 59 deletions(-)

diff --git a/merlin-driver/src/main/java/gov/nasa/jpl/aerie/merlin/driver/engine/SimulationEngine.java b/merlin-driver/src/main/java/gov/nasa/jpl/aerie/merlin/driver/engine/SimulationEngine.java
index 6f77878ec7..efbddb54f2 100644
--- a/merlin-driver/src/main/java/gov/nasa/jpl/aerie/merlin/driver/engine/SimulationEngine.java
+++ b/merlin-driver/src/main/java/gov/nasa/jpl/aerie/merlin/driver/engine/SimulationEngine.java
@@ -73,29 +73,7 @@ public final class SimulationEngine implements AutoCloseable {
   private final Map<TaskId, Set<TaskId>> taskChildren = new HashMap<>();
 
   /** A thread pool that modeled tasks can use to keep track of their state between steps. */
-  private final ExecutorService executor = getLoomOrFallback();
-
-  private static ExecutorService getLoomOrFallback() {
-    // Try to use Loom's lightweight virtual threads, if possible. Otherwise, just use a thread pool.
-    // This approach is inspired by that of Javalin 5.
-    // https://github.com/javalin/javalin/blob/97e9e23ebe8f57aa353bc7a45feb560ad61e50a0/javalin/src/main/java/io/javalin/util/ConcurrencyUtil.kt#L48-L51
-    try {
-      // Use reflection to avoid needing `--enable-preview` at compile-time.
-      // If the runtime JVM is run with `--enable-preview`, this should succeed.
-      return (ExecutorService) Executors.class.getMethod("newVirtualThreadPerTaskExecutor").invoke(null);
-    } catch (final ReflectiveOperationException ex) {
-      return Executors.newCachedThreadPool($ -> {
-        final var t = new Thread($);
-        // TODO: Make threads non-daemons.
-        //  We're marking these as daemons right now solely to ensure that the JVM shuts down cleanly in lieu of
-        //  proper model lifecycle management.
-        //  In fact, daemon threads can mask bad memory leaks: a hanging thread is almost indistinguishable
-        //  from a dead thread.
-        t.setDaemon(true);
-        return t;
-      });
-    }
-  }
+  private final ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor();
 
   /** Schedule a new task to be performed at the given time. */
   public <Return> TaskId scheduleTask(final Duration startTime, final TaskFactory<Return> state) {
diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/ResumableSimulationTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/ResumableSimulationTest.java
index 0f2bd23508..4f40990a76 100644
--- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/ResumableSimulationTest.java
+++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/ResumableSimulationTest.java
@@ -2,7 +2,6 @@
 
 import gov.nasa.jpl.aerie.merlin.driver.ActivityDirectiveId;
 import gov.nasa.jpl.aerie.merlin.driver.SerializedActivity;
-import gov.nasa.jpl.aerie.merlin.driver.engine.SimulationEngine;
 import gov.nasa.jpl.aerie.merlin.protocol.types.Duration;
 import gov.nasa.jpl.aerie.scheduler.SchedulingInterruptedException;
 import gov.nasa.jpl.aerie.scheduler.SimulationUtility;
@@ -12,7 +11,6 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Map;
-import java.util.concurrent.ThreadPoolExecutor;
 
 import static gov.nasa.jpl.aerie.merlin.protocol.types.Duration.SECONDS;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -93,40 +91,6 @@ public void testStopsAtEndOfPlanningHorizon() throws SchedulingInterruptedExcept
     assert(resumableSimulationDriver.getSimulationResults(Instant.now()).unfinishedActivities.size() == 1);
   }
 
-  @Test
-  public void testThreadsReleased() throws SchedulingInterruptedException {
-    final var activity = new TestSimulatedActivity(
-        Duration.of(0, SECONDS),
-        new SerializedActivity("BasicActivity", Map.of()),
-        new ActivityDirectiveId(1));
-    final var fooMissionModel = SimulationUtility.getFooMissionModel();
-    resumableSimulationDriver = new ResumableSimulationDriver<>(fooMissionModel, tenHours, ()-> false);
-    try (final var executor = unsafeGetExecutor(resumableSimulationDriver)) {
-      for (var i = 0; i < 20000; i++) {
-        resumableSimulationDriver.initSimulation();
-        resumableSimulationDriver.clearActivitiesInserted();
-        resumableSimulationDriver.simulateActivity(activity.start, activity.activity, null, true, activity.id);
-        assertTrue(
-            executor.getActiveCount() < 100,
-            "Threads are not being cleaned up properly - this test shouldn't need more than 2 threads, but it used at least 100");
-      }
-    }
-  }
-
-  private static ThreadPoolExecutor unsafeGetExecutor(final ResumableSimulationDriver<?> driver) {
-    try {
-      final var engineField = ResumableSimulationDriver.class.getDeclaredField("engine");
-      engineField.setAccessible(true);
-
-      final var executorField = SimulationEngine.class.getDeclaredField("executor");
-      executorField.setAccessible(true);
-
-      return (ThreadPoolExecutor) executorField.get(engineField.get(driver));
-    } catch (final ReflectiveOperationException ex) {
-      throw new RuntimeException(ex);
-    }
-  }
-
   private ArrayList<TestSimulatedActivity> getActivities(){
     final var acts = new ArrayList<TestSimulatedActivity>();
     var act1 = new TestSimulatedActivity(