-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
JVM finalizer threads get adopted into JRuby thread list #4960
Comments
JRuby's list of threads may include those that come from outside JRuby itseld, like JVM finalizer or worker threads. Because this might happen at some unknown time in the future, it causes tests relying on Thread.list to fail sporadically. # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch jruby-9.1 # Your branch is up-to-date with 'origin/jruby-9.1'. # # Changes to be committed: # modified: test/mri/excludes/TestThread.rb # # Untracked files: # bench1.rb # bench2.rb # blah # blah.rb # blah.sh # blah2.rb # jruby_jetpdb/ # link20180108-44247-163yxpr # nic.rb # ripper_bug_4792.rb # run_jruby.sh # test/jruby/tcttac37356-0.jpg # test/jruby/testapp/testapp # test_thr_kill_count # testapp2/ # tmp/ # var/ # "\316\261.rb" # "\343\201\202" #
Ah this is a cool find. There is a duality which is difficult to resolve in adoption. If you are an embedder you may think it should be in this list because you made it. OTOH for a case like finalization I suspect no one expects to see it except by experience of having noticed it in passing (like you did). So in the first case it should be in the list as the user thinks it is a Ruby thread. In the later case I do not think people would expect that to show up; Although perhaps there should be a way of seeing that from Ruby space? Special casing adoption due to finalization as an implicit impl-specific backend thing and not displaying does not feel cookie cutter to me. At least not from the explanation I gave above. It is possible we maybe even make more background impl-specific threads which may end up doing callMethod but I really don't see those as showing up in Thread.list either? |
Well there's a few places where we'll see threads that you wouldn't normally see in MRI:
The problem I see is that it's hard to manage the thread context that goes with a given adopted thread unless we put it in the same structures we build Thread.list from. Of course, there's no reason we can't flag external threads as "adopted" and not include them when we build the list. Bottom line: we can't get rid of the adoption plumbing, but we may be able to hide it. Another option to reduce the risk of losing threads people expect to see: we could have this behavior on only when running from command line (i.e. through org.jruby.Main). Rubyists would be far less likely to complain. Still another possibility: threads spun up by the JVM or by third-party code will have their own thread group. We could apply a runtime-specific thread group to all JRuby threads, and then we'd always know which are ours and which aren't. We can present that in Thread.list however is appropriate. |
This may or may not be a real problem, but it causes many tests to fail sporadically, such as this one from MRI's test_thread.rb which tests that Thread.list has the correct number of threads in it:
There are also tests in RubySpec that occasionally see extra threads in lists.
After some exploration today I figured out that in the case of the MRI test above (and likely in many of the other cases) the extra thread(s) are from the JVM's finalization subsystem. The JVM runs object finalization in a VM-started thread that enumerates all queued up soon-to-be-dead references and runs the finalizers associated with those objects. If those finalizers need to run Ruby code, the finalizer thread will be "adopted" into JRuby, showing up in
Thread.list
among other places.This causes the tests to fail sporadically, since sometimes finalization has started and sometimes it has not; sometimes finalization needs to interact with Ruby and sometimes not.
I'm not sure if there's a fix for this. It is not unreasonable for us to have extra VM threads; Thread.list expectations imply that user code is the only thing allowed to start threads. And any thread that runs Ruby code does need to be adopted.
My thought, at the moment, is that perhaps including all adopted threads in Thread.list is not appropriate. With some tweaking, those threads could be marked as such so they would not be included. This would potentially break embedding users that expect non-JRuby threads to be in the list, but providing an alternative mechanism may be enough.
Alternatively, if we can determine a thread is from the JVM finalization subsystem, we could exclude just that one thread. But this feels too cookie-cutter to me.
There's also the looming requirement that we move away from finalization.
Object.finalize
is deprecated as of Java 9, with the preferred alternative likely being phantom references and a userland finalization thread. Again, we might have a VM-level thread showing up in the Ruby thread list.The text was updated successfully, but these errors were encountered: