Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RuntimeManagementTest is not shutting down Threads properly #10804

Closed
hubertp opened this issue Aug 13, 2024 · 9 comments · Fixed by #10890
Closed

RuntimeManagementTest is not shutting down Threads properly #10804

hubertp opened this issue Aug 13, 2024 · 9 comments · Fixed by #10890
Assignees
Labels
-compiler -libs Libraries: New libraries to be implemented

Comments

@hubertp
Copy link
Collaborator

hubertp commented Aug 13, 2024

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.

@hubertp hubertp added -compiler -libs Libraries: New libraries to be implemented labels Aug 13, 2024
@JaroslavTulach
Copy link
Member

JaroslavTulach commented Aug 23, 2024

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
Copy link
Member

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
Copy link
Member

JaroslavTulach commented Aug 24, 2024

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
Copy link
Member

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
Copy link
Member

Culprit: RuntimeManagementTest doesn't close the EnsoContext!

@enso-bot
Copy link

enso-bot bot commented Aug 27, 2024

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

Progress: - note about run a node in different context: #10719 (comment)

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

@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Aug 27, 2024
@JaroslavTulach
Copy link
Member

Culprit: RuntimeManagementTest doesn't close the EnsoContext!

At the end

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

@enso-bot
Copy link

enso-bot bot commented Aug 28, 2024

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

Progress: - minimizing moving parts: 3c28bc9

@enso-bot
Copy link

enso-bot bot commented Aug 29, 2024

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

Progress: .

GitHub
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 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-compiler -libs Libraries: New libraries to be implemented
Projects
Archived in project
2 participants