Reducing memory leaks in runtime-integration-tests (#10793)

Inspired by the revert done in #10778, started looking into apparent memory leaks in `runtime-integration-tests`, written in Java. Initial state: ![Screenshot from 2024-08-09 14-36-29](https://github.com/user-attachments/assets/39abd48f-503b-49d8-af97-da051352c70d) After: ![Screenshot from 2024-08-12 16-33-40](https://github.com/user-attachments/assets/2bf4cc2d-7e0e-4d22-8810-c2e7e5c3b065) # Important Notes Some remaining issues: - [ ] [TCK tests](https://github.com/enso-org/enso/tree/develop/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/tck) appear to have some memory leaks but we are essentially enabling them via simple inheritance - [ ] [RuntimeManagementTest](https://github.com/enso-org/enso/blob/develop/engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala) appears to be broken as it doesn't seem to shutdown properly created threads: ![Screenshot from 2024-08-12 16-29-09](https://github.com/user-attachments/assets/d90aca62-0562-4287-88b7-6d4719e5cf50) Leaving this for now, as it will probably need to be taken care by initial authors of those tests, if possible. Plus this PR leaves tests in a much better state than before.
enso-org · Aug 13, 2024 · ff7e31c · ff7e31c
1 parent 31e589e
commit ff7e31c
Show file tree

Hide file tree

Showing 64 changed files with 364 additions and 206 deletions.
diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/PackageRepository.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/PackageRepository.scala
@@ -105,6 +105,7 @@ trait PackageRepository {
     context: CompilerContext
   ): Option[IRModule]
 
+  def shutdown(): Unit
 }
 
 object PackageRepository {

diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExecStrictCompilerTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExecStrictCompilerTest.java
@@ -17,8 +17,8 @@
 import org.graalvm.polyglot.PolyglotException;
 import org.graalvm.polyglot.Source;
 import org.graalvm.polyglot.io.IOAccess;
+import org.junit.After;
 import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -45,14 +45,15 @@ public static void initEnsoContext() {
     assertNotNull("Enso language is supported", ctx.getEngine().getLanguages().get("enso"));
   }
 
-  @Before
+  @After
   public void cleanMessages() {
     MESSAGES.reset();
   }
 
   @AfterClass
   public static void closeEnsoContext() {
     ctx.close();
+    ctx = null;
   }
 
   @Test

diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExportedSymbolsTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/ExportedSymbolsTest.java
@@ -47,13 +47,14 @@ public void exportedSymbolsFromSingleModule() throws IOException {
         type A_Type
         """);
     ProjectUtils.createProject("Proj", Set.of(mainSrcMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(1));
-    assertThat(mainExportedSymbols.containsKey("A_Type"), is(true));
-    assertThat(
-        mainExportedSymbols.get("A_Type").get(0), instanceOf(BindingsMap.ResolvedType.class));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(1));
+      assertThat(mainExportedSymbols.containsKey("A_Type"), is(true));
+      assertThat(
+          mainExportedSymbols.get("A_Type").get(0), instanceOf(BindingsMap.ResolvedType.class));
+    }
   }
 
   @Test
@@ -70,11 +71,12 @@ public void transitivelyExportedSymbols() throws IOException {
         type B_Type
         """);
     ProjectUtils.createProject("Proj", Set.of(aMod, mainSrcMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(2));
-    assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Type", "B_Type"));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(2));
+      assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Type", "B_Type"));
+    }
   }
 
   @Test
@@ -91,11 +93,12 @@ public void exportSymbolFromDifferentModule() throws IOException {
         type B_Type
         """);
     ProjectUtils.createProject("Proj", Set.of(mainMod, bMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(2));
-    assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Type", "B_Type"));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(2));
+      assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Type", "B_Type"));
+    }
   }
 
   @Test
@@ -111,11 +114,12 @@ public void exportRenamedSymbol() throws IOException {
         export project.A_Module.A_Type as Foo
         """);
     ProjectUtils.createProject("Proj", Set.of(aMod, mainSrcMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(1));
-    assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("Foo"));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(1));
+      assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("Foo"));
+    }
   }
 
   @Test
@@ -133,13 +137,16 @@ public void exportedSymbolsFromSubModule() throws IOException {
         import project.Synthetic_Module
         """);
     ProjectUtils.createProject("Proj", Set.of(aMod, mainMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var syntheticModExpSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Synthetic_Module");
-    assertThat(
-        "Just a A_Module submodule should be exported", syntheticModExpSymbols.size(), is(1));
-    assertThat(
-        "Just a A_Module submodule should be exported", syntheticModExpSymbols, hasKey("A_Module"));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var syntheticModExpSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Synthetic_Module");
+      assertThat(
+          "Just a A_Module submodule should be exported", syntheticModExpSymbols.size(), is(1));
+      assertThat(
+          "Just a A_Module submodule should be exported",
+          syntheticModExpSymbols,
+          hasKey("A_Module"));
+    }
   }
 
   @Test
@@ -156,14 +163,16 @@ public void exportTypeFromModuleWithSameName() throws IOException {
         export project.A_Module.A_Module
         """);
     ProjectUtils.createProject("Proj", Set.of(aMod, mainMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(1));
-    assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Module"));
-    assertThat(mainExportedSymbols.get("A_Module").size(), is(1));
-    assertThat(
-        mainExportedSymbols.get("A_Module").get(0), is(instanceOf(BindingsMap.ResolvedType.class)));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(1));
+      assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Module"));
+      assertThat(mainExportedSymbols.get("A_Module").size(), is(1));
+      assertThat(
+          mainExportedSymbols.get("A_Module").get(0),
+          is(instanceOf(BindingsMap.ResolvedType.class)));
+    }
   }
 
   @Test
@@ -179,15 +188,16 @@ public void exportModuleWithTypeWithSameName() throws IOException {
         export project.A_Module
         """);
     ProjectUtils.createProject("Proj", Set.of(aMod, mainMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(1));
-    assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Module"));
-    assertThat(mainExportedSymbols.get("A_Module").size(), is(1));
-    assertThat(
-        mainExportedSymbols.get("A_Module").get(0),
-        is(instanceOf(BindingsMap.ResolvedModule.class)));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(1));
+      assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("A_Module"));
+      assertThat(mainExportedSymbols.get("A_Module").size(), is(1));
+      assertThat(
+          mainExportedSymbols.get("A_Module").get(0),
+          is(instanceOf(BindingsMap.ResolvedModule.class)));
+    }
   }
 
   @Test
@@ -205,15 +215,16 @@ public void exportSyntheticModule() throws IOException {
         export project.Synthetic_Module
         """);
     ProjectUtils.createProject("Proj", Set.of(aMod, mainMod), projDir);
-    var ctx = createCtx(projDir);
-    compile(ctx);
-    var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
-    assertThat(mainExportedSymbols.size(), is(1));
-    assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("Synthetic_Module"));
-    assertThat(mainExportedSymbols.get("Synthetic_Module").size(), is(1));
-    assertThat(
-        mainExportedSymbols.get("Synthetic_Module").get(0),
-        is(instanceOf(BindingsMap.ResolvedModule.class)));
+    try (var ctx = createCtx(projDir)) {
+      compile(ctx);
+      var mainExportedSymbols = getExportedSymbolsFromModule(ctx, "local.Proj.Main");
+      assertThat(mainExportedSymbols.size(), is(1));
+      assertThat(mainExportedSymbols.keySet(), containsInAnyOrder("Synthetic_Module"));
+      assertThat(mainExportedSymbols.get("Synthetic_Module").size(), is(1));
+      assertThat(
+          mainExportedSymbols.get("Synthetic_Module").get(0),
+          is(instanceOf(BindingsMap.ResolvedModule.class)));
+    }
   }
 
   private static Context createCtx(Path projDir) {

diff --git a/...ntime-integration-tests/src/test/java/org/enso/compiler/ImportsAndFQNConsistencyTest.java b/...ntime-integration-tests/src/test/java/org/enso/compiler/ImportsAndFQNConsistencyTest.java
@@ -103,6 +103,7 @@ public static void initCtx() {
   @AfterClass
   public static void disposeCtx() {
     ctx.close();
+    ctx = null;
   }
 
   private final Symbol symbol;

diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/SerdeCompilerTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/SerdeCompilerTest.java
@@ -74,7 +74,7 @@ private void parseSerializedModule(String projectName, String forbiddenMessage)
         var persisted = f.get(10, TimeUnit.SECONDS);
         assertEquals("Fib_Test library has been fully persisted", true, persisted);
       }
-      old = module.getIr();
+      old = module.getIr().duplicate(true, true, true, false);
       ctx.leave();
     }
 
@@ -103,7 +103,7 @@ private void parseSerializedModule(String projectName, String forbiddenMessage)
       var mainValue = ctx.asValue(main);
       assertEquals(42, mainValue.execute().asInt());
 
-      now = module.getIr();
+      now = module.getIr().duplicate(true, true, true, false);
 
       ctx.leave();
     }

diff --git a/...e/runtime-integration-tests/src/test/java/org/enso/compiler/SerializationManagerTest.java b/...e/runtime-integration-tests/src/test/java/org/enso/compiler/SerializationManagerTest.java
@@ -28,11 +28,12 @@ public class SerializationManagerTest {
 
   private static final long COMPILE_TIMEOUT_SECONDS = 20;
 
-  private final PackageManager<TruffleFile> packageManager;
-  private final InterpreterContext interpreterContext;
-  private final EnsoContext ensoContext;
+  private PackageManager<TruffleFile> packageManager;
+  private InterpreterContext interpreterContext;
+  private EnsoContext ensoContext;
 
-  public SerializationManagerTest() {
+  @Before
+  public void setup() {
     packageManager = new PackageManager<>(new TruffleFileSystem());
     interpreterContext = new InterpreterContext(x -> x);
     ensoContext =
@@ -41,22 +42,20 @@ public SerializationManagerTest() {
             .getBindings(LanguageInfo.ID)
             .invokeMember(MethodNames.TopScope.LEAK_CONTEXT)
             .asHostObject();
-  }
-
-  @Before
-  public void setup() {
     interpreterContext.ctx().initialize(LanguageInfo.ID);
     interpreterContext.ctx().enter();
   }
 
   @After
   public void teardown() {
-    interpreterContext.ctx().close();
+    interpreterContext.close();
+    ensoContext.shutdown();
+    ensoContext = null;
   }
 
   private Path getLibraryPath(LibraryName libraryName) {
     return Paths.get(
-            interpreterContext.languageHome(),
+            interpreterContext.languageHome().toFile().getAbsolutePath(),
             "..",
             "lib",
             libraryName.namespace(),

diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/SerializerTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/SerializerTest.java
@@ -42,29 +42,29 @@ public void testSerializationOfFQNs() throws Exception {
     var pkgPath = new File(getClass().getClassLoader().getResource(testName).getPath());
     var pkg = PackageManager.Default().fromDirectory(pkgPath).get();
 
-    var ctx = ensoContextForPackage(testName, pkgPath);
-    var ensoContext =
-        (EnsoContext)
-            ctx.getBindings(LanguageInfo.ID)
-                .invokeMember(MethodNames.TopScope.LEAK_CONTEXT)
-                .asHostObject();
-    var mainModuleOpt = ensoContext.getModuleForFile(pkg.mainFile());
-    assertEquals(mainModuleOpt.isPresent(), true);
+    try (var ctx = ensoContextForPackage(testName, pkgPath)) {
+      var ensoContext =
+          (EnsoContext)
+              ctx.getBindings(LanguageInfo.ID)
+                  .invokeMember(MethodNames.TopScope.LEAK_CONTEXT)
+                  .asHostObject();
+      var mainModuleOpt = ensoContext.getModuleForFile(pkg.mainFile());
+      assertEquals(mainModuleOpt.isPresent(), true);
 
-    var compiler = ensoContext.getCompiler();
-    var module = mainModuleOpt.get().asCompilerModule();
+      var compiler = ensoContext.getCompiler();
+      var module = mainModuleOpt.get().asCompilerModule();
 
-    ctx.enter();
-    var result = compiler.run(module);
-    assertEquals(result.compiledModules().exists(m -> m == module), true);
-    var useThreadPool = compiler.context().isCreateThreadAllowed();
-    var future = compiler.context().serializeModule(compiler, module, true, useThreadPool);
-    var serialized = future.get(5, TimeUnit.SECONDS);
-    assertEquals(serialized, true);
-    var deserialized = compiler.context().deserializeModule(compiler, module);
-    assertTrue("Deserialized", deserialized);
-    compiler.context().shutdown(true);
-    ctx.leave();
-    ctx.close();
+      ctx.enter();
+      var result = compiler.run(module);
+      assertEquals(result.compiledModules().exists(m -> m == module), true);
+      var useThreadPool = compiler.context().isCreateThreadAllowed();
+      var future = compiler.context().serializeModule(compiler, module, true, useThreadPool);
+      var serialized = future.get(5, TimeUnit.SECONDS);
+      assertEquals(serialized, true);
+      var deserialized = compiler.context().deserializeModule(compiler, module);
+      assertTrue("Deserialized", deserialized);
+      compiler.context().shutdown(true);
+      ctx.leave();
+    }
   }
 }
diff --git a/...e/runtime-integration-tests/src/test/java/org/enso/compiler/VectorArraySignatureTest.java b/...e/runtime-integration-tests/src/test/java/org/enso/compiler/VectorArraySignatureTest.java
@@ -35,6 +35,7 @@ public static void initEnsoParser() {
   @AfterClass
   public static void closeEnsoParser() throws Exception {
     ensoCompiler.close();
+    ensoCompiler = null;
   }
 
   @Test

diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/IRDumpTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/IRDumpTest.java
@@ -19,8 +19,7 @@ public void testIrDump() {
     System.setProperty(IRDumper.SYSTEM_PROP, "true");
     try (var ctx = ContextUtils.defaultContextBuilder().out(out).build()) {
       // Dumping is done in the compiler, so it is enough just to compile the module
-      var moduleIr =
-          ContextUtils.compileModule(ctx, """
+      ContextUtils.compileModule(ctx, """
           main = 42
           """, "MyMainModule");
       assertThat(
@@ -39,6 +38,7 @@ public void testIrDump() {
       } catch (IOException e) {
         // Ignore. The ir-dumps directory should be deleted eventually.
       }
+      out.reset();
     }
   }
 }
diff --git a/.../runtime-integration-tests/src/test/java/org/enso/interpreter/caches/ModuleCacheTest.java b/.../runtime-integration-tests/src/test/java/org/enso/interpreter/caches/ModuleCacheTest.java
@@ -14,6 +14,7 @@
 import org.enso.test.utils.ContextUtils;
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.Source;
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -30,6 +31,12 @@ public static void initializeContext() throws Exception {
             .build();
   }
 
+  @AfterClass
+  public static void disposeContext() {
+    ctx.close();
+    ctx = null;
+  }
+
   @Test
   public void testCompareList() throws Exception {
     var ensoCtx =

diff --git a/...egration-tests/src/test/java/org/enso/interpreter/node/callable/PolyglotCallTypeTest.java b/...egration-tests/src/test/java/org/enso/interpreter/node/callable/PolyglotCallTypeTest.java
@@ -27,6 +27,7 @@ public static void setupCtx() {
   @AfterClass
   public static void closeCtx() {
     ctx.close();
+    ctx = null;
   }
 
   @Test

diff --git a/...tests/src/test/java/org/enso/interpreter/node/expression/builtin/meta/TypeOfNodeTest.java b/...tests/src/test/java/org/enso/interpreter/node/expression/builtin/meta/TypeOfNodeTest.java
@@ -66,6 +66,7 @@ public static Object[][] allPossibleEnsoInterpreterValues() throws Exception {
   public static void disposeCtx() throws Exception {
     if (ctx != null) {
       ctx.close();
+      ctx = null;
     }
   }