Remove running threads on tearDown #3316
Conversation
|
This looks reasonable to me, but what do I know :) |
|
Oh, is this sufficient for #3313? I thought you would have expected JRuby to actually kill those threads, which I wasn't willing to do (and might not even be able to do, safely). |
|
Hey Charles, Thanks for the response. I'm not %100 sure how I would "kill" threads. My test indicates that the threads are no longer running after calling tearDown so I was assuming that the patch was killing threads. Is there a better way to do this? As for safely my thoughts are that tearDown is the same as killing a ruby process. Developers working with MRI seem to be starting threads with no intent of stopping them safely. Mongo's new ruby driver is an example of this, they start a heartbeat thread that never dies. |
|
@stewartmatheson It's possible to iterate over all threads and kill them now, using Ruby.getThreadService().getActiveRubyThreads(). Note that this will usually only give you the threads initiated from within that JRuby instance, but I think that's what you want. For the record, I do not believe our Ruby.teardown should forcibly kill off user threads. We've actually had complains that teardown does too much in cases where people just allow the main thread to terminate but expect their other threads to keep running. We can't please everyone here. However, I think for embedding purposes it's worth adding a new mechanism to "teardown and stop threads". This might be the default behavior for ScriptingContainer, but that's debatable. Probably not going to get this until major work in 9.2. Help wanted! |
This change clears the following on JRuby runtime teardown: * The thread-local context reference in ThreadService (dereferenced entirely) * The mainContext reference in ThreadService * The mapping from native threads to RubyThread instances. This should clear all JRuby-specific runtime data attached to both Ruby and adopted Java threads. Fixes jruby#3313. Supersedes jruby#3316.
This change clears the following on JRuby runtime teardown: * The thread-local context reference in ThreadService (dereferenced entirely) * The mainContext reference in ThreadService * The mapping from native threads to RubyThread instances. This should clear all JRuby-specific runtime data attached to both Ruby and adopted Java threads. Fixes jruby#3313. Supersedes jruby#3316.
This has come up many times; Ruby threads are daemon threads by default, which means they will not keep the main script from exiting. As a result, if there's any threads still runnign when the main script exits, we may tear down resources they need to run properly. Even though they're about to die, they may produce errors on the way out as shown in jruby#5909 (comment) and other bug reports over the years like jruby#5519, jruby#3316, jruby#3313 and others. This is not a fix for the behavior, but it introduces a non-verbose warning if the JRuby runtime is torn down while there's still active threads.
See #3313 .