enso-org / enso

Hybrid visual and textual functional programming.
https://ensoanalytics.com
Apache License 2.0
7.36k stars 324 forks source link

RuntimeManagementTest is not shutting down Threads properly #10804

Closed hubertp closed 1 month ago

hubertp commented 2 months ago

As revealed in the investigation related to memory leaks.

Seems like every time RuntimeManagementTest is run, we keep two threads up and running: Screenshot from 2024-08-12 16-29-09

Not sure if this is caused by the bug in a test or in the implementation (finalizers) of ManagedResource.

JaroslavTulach commented 2 months ago

Possible fix to give unnamed threads some name doesn't help as this test isn't leaking the threads:

$ git diff
diff --git engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala
index 876b42ab20..7f3b018f3a 100644
--- engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala
+++ engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/semantic/RuntimeManagementTest.scala
@@ -47,7 +47,7 @@ class RuntimeManagementTest extends InterpreterTest {
       }

       def runTest(n: Int = 5): Unit = {
-        val threads = 0.until(n).map(_ => new Thread(runnable))
+        val threads = 0.until(n).map(_ => new Thread(runnable, "RuntimeManagementTest-Thread"))
         threads.foreach(_.start())
         var reportedCount = 0
         while (reportedCount < n) {

that names one of the testing threads.

JaroslavTulach commented 2 months ago

Seems like every time RuntimeManagementTest is run, we keep two threads up and running:

What do you mean. "Every time" it is run? I can:

sbt:runtime-integration-tests> testOnly *RuntimeManagementTest

however that runs the test only once. How do I reproduce "every time"?

JaroslavTulach commented 2 months ago

Taking heap dump shows the Thread-144 hanging there after RuntimeManagementTest gets executed in a sequence of tests started by

sbt:runtime-integration-tests> test

is a LanguageSystemThread:

language system thread #2

The Thread-143 is also LanguageSystemThread. Nobody has references to them. They point to the same instance of PolyglotImpl via the polyglot field. And to two different instances of PolyglotContextImpl via the polyglotContext field.

JaroslavTulach commented 2 months ago

There are four tests in RuntimeManagementTest and only the last three are leaving the threads behind. The first one creates new suspicious threads:

[info] RuntimeManagementTest:
[info] Enso Code Execution
[info]   when Context is Cached
SLF4J: Attempting to load provider "org.enso.logger.TestLogProvider" specified via "slf4j.provider" system property
[info]   - should Interrupt threads through Thread#interrupt() (1 second, 812 milliseconds)
[info]   - should Automatically free managed resources !!! IGNORED !!!
[info]   - should Automatically free managed resources amongst manual closure of other managed resources !!! IGNORED !!!
[info]   - should Automatically free managed resources amongst manual takeover of other managed resources !!! IGNORED !!!
[info] Enso Code Execution
[info]   when Context is Uncached
[info]   - should Interrupt threads through Thread#interrupt() (326 milliseconds)
[info]   - should Automatically free managed resources !!! IGNORED !!!
[info]   - should Automatically free managed resources amongst manual closure of other managed resources !!! IGNORED !!!
[info]   - should Automatically free managed resources amongst manual takeover of other managed resources !!! IGNORED !!!
Found 0 suspicious threads
[info] Test run org.enso.interpreter.test.instrument.RuntimeManagementCleanupTest finished: 0 failed, 0 ignored, 1 total, 1.057s
JaroslavTulach commented 2 months ago

Culprit: RuntimeManagementTest doesn't close the EnsoContext!

enso-bot[bot] commented 1 month ago

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-26):

Progress: - note about run a node in different context: https://github.com/enso-org/enso/issues/10719#issuecomment-2309747331

Premier Support | Oracle
Elevate your Oracle experience with Premier Support for risk mitigation, cost reduction, and comprehensive system protection. Get expert 24/7 service and security.
JaroslavTulach commented 1 month ago

Culprit: RuntimeManagementTest doesn't close the EnsoContext!

At the end

fixes the problem by finishing the threads when they become idle.

enso-bot[bot] commented 1 month ago

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-27):

Progress: - minimizing moving parts: https://github.com/enso-org/enso/pull/10890/commits/3c28bc975e50f075a726d47e2cc8dbfbd7f2ef94

enso-bot[bot] commented 1 month ago

Jaroslav Tulach reports a new STANDUP for yesterday (2024-08-28):

Progress: .

GitHub
Benchmark measuring startup with import of all libraries by JaroslavTulach · Pull Request #10899 · enso-org/enso
Pull Request Description Introduces new startup "Hello World" benchmark with many imports and a performance improvement by delaying loading classes in registerPolyglotSymbol. Checklist Pl...
Discord
Discord - Group Chat That’s All Fun & Games
Discord is great for playing games and chilling with friends, or even building a worldwide community. Customize your own space to talk, play, and hang out.