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

Use of deprecated Thread.stop() causes ThreadDeath exceptions propagating to caller #1183

Closed
headius opened this Issue Oct 29, 2013 · 16 comments

Comments

Projects
None yet
9 participants
@headius
Copy link
Member

headius commented Oct 29, 2013

This was originally at: https://jira.codehaus.org/browse/JRUBY-7074

A change introduced in jruby 1.6.8 is a call (well, three calls) to the deprecated Thread.stop() method in org.jruby.util.ShellLauncher.handleStreams(). This causes a problem in an embedded interpreter that runs ruby scripts which in turn start a shell due to ThreadDeath exceptions being propagated up into the embedding code. Should the jruby framework itself be catching and dealing with any cleanup resulting from these exceptions? Or perhaps the use of deprecated methods should be reviewed?

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Oct 29, 2013

We should be handling those stop exceptions and not allowing them to get out into Ruby proper.

@strongriley

This comment has been minimized.

Copy link

strongriley commented Nov 12, 2013

👍

@inger

This comment has been minimized.

Copy link

inger commented Jan 31, 2014

Until this is fixed, any suggested workaround to try in my Ruby code?
E.g just rescue'ing ThreadDeathErrors around system,`` calls?

@headius headius modified the milestones: JRuby 1.7.12, JRuby 1.7.10 Feb 21, 2014

@enebo enebo modified the milestones: JRuby 1.7.13, JRuby 1.7.12 Apr 15, 2014

@enebo enebo modified the milestones: JRuby 1.7.14, JRuby 1.7.13 Jun 24, 2014

@enebo enebo modified the milestones: JRuby 1.7.14, JRuby 1.7.15 Aug 27, 2014

@binaryplease

This comment has been minimized.

Copy link

binaryplease commented May 5, 2015

Experiencing the same issue here.

My setup

binaryplease➜~(master✗)» ruby -v                                                                                                                                                                                                     
jruby 1.7.19 (1.9.3p551) 2015-02-21 32f5af0 on OpenJDK 64-Bit Server VM 1.8.0_45-b14 +jit [linux-amd64]

binaryplease➜~(master✗)» uname -a                                                                                                                                                                                                    
Linux binaryplease-laptop 4.0.1-1-ARCH #1 SMP PREEMPT Wed Apr 29 12:00:26 CEST 2015 x86_64 GNU/Linux

binaryplease➜~(master✗)» java -version                                                                                                                                                                                               
openjdk version "1.8.0_45"
OpenJDK Runtime Environment (build 1.8.0_45-b14)
OpenJDK 64-Bit Server VM (build 25.45-b02, mixed mode)
@jsvd

This comment has been minimized.

Copy link

jsvd commented Feb 9, 2016

I'm not sure if this is the same problem but I can reproduce thread death quite consistently:

% ruby -v
jruby 1.7.23 (1.9.3p551) 2015-11-24 f496dd5 on Java HotSpot(TM) 64-Bit Server VM 1.8.0_20-b26 +jit [darwin-x86_64]
% ruby -e "loop { system(\"echo 'hey' > /tmp/a\") }"

Exception: java.lang.ThreadDeath thrown from the UncaughtExceptionHandler in thread "Thread-1279"

Exception: java.lang.ThreadDeath thrown from the UncaughtExceptionHandler in thread "Thread-1321"

Exception: java.lang.ThreadDeath thrown from the UncaughtExceptionHandler in thread "Thread-1327"
@kares

This comment has been minimized.

Copy link
Member

kares commented Feb 12, 2016

jruby-1_7 branch started failing on travis-ci with these lately e.g. :

probably smt changed on travis' end (re-running a green build gets red) but still seems like there might be something to fix on JRuby's end ... note that 9K builds do not seem to exhibit the behavior.

@kares kares modified the milestones: JRuby 1.7.25, JRuby 1.7.15 Feb 12, 2016

@stevenschlansker

This comment has been minimized.

Copy link

stevenschlansker commented Mar 29, 2016

This also affects our rake-invoked Spec tests of our Puppet code under JRuby.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Apr 12, 2016

I'll have a look. Probably fixed in 9k.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Apr 12, 2016

Ok, I can reproduce the message...but it isn't actually killing anything. The exception occurs within threads that are intended to die a horrible, flaming death. I suspect the problem is just that we don't have a default exception handler on these threads, and the ThreadDeath fires outside the exception handling bits.

There's a couple options...none of them super great:

  1. We could avoid using Thread#stop altogether. This would obviously eliminate the output, but it might mean some threads that never wake up from a blocking IO call get stuck and accumulate. The original commit that added some of these stop calls mentions a JIRA issue we can't see anymore, but it appears this thread leak was a problem.
  2. We may be able to just add enough handling to make this message go away. This might just need better try/catch logic, or it may mean we need to install an "uncaught exception handler" for these threads that just quietly ignores failures (like all the other code in there). The risk is that we are still calling stop and shouldn't be.

This is not fixed in 9k, but it likely doesn't show up as often because UNIX platforms no longer use this logic for subprocess management. It may be easy to still reproduce it in 9k on Windows.

@stevenschlansker

This comment has been minimized.

Copy link

stevenschlansker commented Apr 12, 2016

For what it's worth, wouldn't a more supported approach be to use Thread#interrupt() to break out of blocking IO operations? It should be possible to do this correctly without usage of Thread#stop()

But maybe doing interrupts correctly is difficult in this code, I obviously don't know the specifics.

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.25 Apr 14, 2016

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.27 Aug 26, 2016

@tobymurray-nanometrics

This comment has been minimized.

Copy link

tobymurray-nanometrics commented Sep 8, 2016

It may be easy to still reproduce it in 9k on Windows.

We use JRuby on Windows and I just for the first time saw the ol' ThreadDeath since switching to the 9k branch. This is not intentionally trying to elicit it, just showing up as unexpected output from one of our scripts. We updated to 9.1.5.0 today, but it may be complete coincidence that it cropped up for the first time.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 8, 2016

I thought I did some work on this. Let me see...

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 8, 2016

Ahh, right...as I mentioned above, this may never happen on a posix system because we use different logic for IO that doesn't require "pump" threads to keep buffered IO streams flowing (a nasty requirement of the JVM's troublesome Process API.

This is further reason why we (read: I) need to bite the bullet and get native IO and process management working on Windows. Let's see how much we can borrow from @djberg96's excellent projects...starting to wonder if we should just throw up our hands and admit he's done it right already.

@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 1.7.27 Sep 8, 2016

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 8, 2016

This needs to go into 9.2 or earlier, but it will never be in 1.7.x.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented Sep 26, 2016

Link with #4112.

@headius

This comment has been minimized.

Copy link
Member Author

headius commented May 15, 2018

This has become less of a priority since most subprocess launching uses native posix_spawn logic now. I still think we need to revisit it but it won't be done for 9.2.0.0.

@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 15, 2018

headius added a commit that referenced this issue Sep 19, 2018

Remove remaining JDK Thread#stop calls for #1183.
These stop calls were put in place during a time when the "pump"
threads were not easily interrupted. Usually this was due to
being forced to work with opaque IO streams rather than the actual
channels. At this point, with Thread#stop going away and still
causing log noise, we have decided to remove all calls.

This code was used primarily by the few remaining non-popen-based
command launches (usually which read the streams to completion and
do not need to be interrupted), and for much of IO on Windows
(since we have not finished porting native IO logic to Windows).
If the removal of these calls leads to leaky pump threads, we will
find an alternative way or prioritize the completion of native IO
on Windows.

headius added a commit that referenced this issue Sep 19, 2018

Remove remaining JDK Thread#stop calls for #1183.
These stop calls were put in place during a time when the "pump"
threads were not easily interrupted. Usually this was due to
being forced to work with opaque IO streams rather than the actual
channels. At this point, with Thread#stop going away and still
causing log noise, we have decided to remove all calls.

This code was used primarily by the few remaining non-popen-based
command launches (usually which read the streams to completion and
do not need to be interrupted), and for much of IO on Windows
(since we have not finished porting native IO logic to Windows).
If the removal of these calls leads to leaky pump threads, we will
find an alternative way or prioritize the completion of native IO
on Windows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.