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

Avoid accidentally killing the teardown thread #7351

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

headius
Copy link
Member

@headius headius commented Sep 8, 2022

If the main thread performing the teardown appears earlier in this map, we may skip the teardown of remaining threads. Instead, we avoid the kill+join dance for the current thread to ensure all other threads get a chance to terminate.

If the main thread performing the teardown appears earlier in this
map, we may skip the teardown of remaining threads. Instead, we
avoid the kill+join dance for the current thread to ensure all
other threads get a chance to terminate.
@headius headius added this to the JRuby 9.3.8.0 milestone Sep 8, 2022
@headius
Copy link
Member Author

headius commented Sep 8, 2022

Relates to #7349 and ruby/timeout#21.

@headius
Copy link
Member Author

headius commented Sep 8, 2022

This is the primary cause of failures on master in the ScriptingContainerTest.testTimeoutCleanup due to the new timeout implementation using a background thread for all timeouts, which is left dangling until process termination. When the teardown thread is prematurely killed, this dangling thread does not get cleaned up and the test fails.

The timeout library should still be making a best effort to shut down its thread gracefully (rather than waiting for it to be forcibly killed) but this change should get the test green on master.

@headius headius merged commit 42a012f into jruby:jruby-9.3 Sep 8, 2022
@headius headius deleted the better_thread_shutdown branch September 8, 2022 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant