Skip to content

Shut down threads on teardown#6176

Merged
headius merged 2 commits intojruby:masterfrom
headius:shutdown_ruby_threads
Apr 14, 2020
Merged

Shut down threads on teardown#6176
headius merged 2 commits intojruby:masterfrom
headius:shutdown_ruby_threads

Conversation

@headius
Copy link
Member

@headius headius commented Apr 14, 2020 •
edited
Loading

This change modifies our shutdown process to asynchronously interrupt and wait for all registered threads (excluding main) to allow them to complete execution.

Previously our behavior was to walk away from running threads. This behavior appears to have either changed in CRuby long ago, or we never implemented this behavior.

Note that this implementation does not currently differentiate between threads started from Ruby code and threads started by other code in the JVM that happened to get adopted by JRuby. We probably should not attempt to kill those adopted threads, and certainly don't want to try to join on them.

The following script illustrates some behavior of thread shutdown. This may be a good start at writing a spec.

# normal Ruby thread should be interrupted and awaited
t = Thread.new {
  p 1
  begin
    sleep
  ensure
    p 2
  end
}

# at_exit blocks should be run after thread termination
at_exit { p 3 }

def four(*); p 4; end

# finalization at shutdown runs after after thread termination
o = Object.new
ObjectSpace.define_finalizer(o, method(:four))

# java daemon thread should not prevent shutdown and also not be interrupted
java.lang.Thread.new do
  java.lang.Thread.currentThread.daemon = true
  begin
    sleep
  ensure
    p 5
  end
end

exit # process should output "1 2 3 4" but not "5".

headius added 2 commits April 14, 2020 14:28
@headius
Shut down threads on teardown
9d9ed80 
This change modifies our shutdown process to asynchronously
interrupt and wait for all registered threads (excluding main) to
allow them to complete execution.

Previously our behavior was to walk away from running threads.
This behavior appears to have either changed in CRuby long ago, or
we never implemented this behavior.

Note that this implementation does not differentiate between
threads started from Ruby code and threads started by other code
in the JVM that happened to get adopted by JRuby. We probably
should not attempt to kill those adopted threads, and certainly
don't want to try to join on them.
@headius
Differentiate adopted threads
1b0d8d4 
This marks all thread created outside of JRuby as "adopted",
meaning that their lifecycles exist outside the lifecycle of the
JRuby runtime. At runtime teardown, only non-adopted threads with
lifecycles tied to the runtime will be terminated.
@headius headius force-pushed the shutdown_ruby_threads branch from 2da902a to 1b0d8d4 Compare April 14, 2020 19:28
@headius headius mentioned this pull request Apr 14, 2020
Closed
@headius headius marked this pull request as ready for review April 14, 2020 20:17
@headius headius mentioned this pull request Apr 14, 2020
Closed
@headius
Copy link
Member Author

headius commented Apr 14, 2020

Note a few issues reference our non-shutdown of running threads:

@headius headius added this to the JRuby 9.3.0.0 milestone Apr 14, 2020
@headius
Copy link
Member Author

headius commented Apr 14, 2020

Reviewed by @enebo with no major concerns. We'll merge and let this bake.

There's one additional change I considered and rejected: don't shut down all threads if we're not the "main" runtime (i.e. check the systemExit argument to Ruby.tearDown). At a glance it seems better to not shut down threads if the process is not exiting, but this would mean we have Ruby threads still running against a torn-down runtime. For now I have opted to always attempt to shutdown child Ruby threads during runtime teardown.

@headius headius merged commit 8c16d88 into jruby:master Apr 14, 2020
@headius headius deleted the shutdown_ruby_threads branch April 14, 2020 20:40
@headius headius mentioned this pull request Sep 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

JRuby 9.3.0.0

Development

Successfully merging this pull request may close these issues.

1 participant

@headius