From 79b6b3382dda5db6b2a016661e782b1d461958ea Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Wed, 19 Sep 2018 13:40:42 -0500 Subject: [PATCH] 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. --- .../java/org/jruby/util/ShellLauncher.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/jruby/util/ShellLauncher.java b/core/src/main/java/org/jruby/util/ShellLauncher.java index 6e0513a8b98..1e9cce2f7ed 100644 --- a/core/src/main/java/org/jruby/util/ShellLauncher.java +++ b/core/src/main/java/org/jruby/util/ShellLauncher.java @@ -1473,7 +1473,7 @@ public void quit() { synchronized (waitLock) { waitLock.notify(); } - stop(); + interrupt(); } } @@ -1523,9 +1523,8 @@ public void run() { } } public void quit() { - interrupt(); this.quit = true; - stop(); + interrupt(); } } @@ -1557,15 +1556,20 @@ private static void handleStreams(Ruby runtime, Process p, InputStream in, Outpu try { pOut.close(); } catch (IOException io) {} try { pErr.close(); } catch (IOException io) {} - // Force t3 to quit, just in case if it's stuck. + // Interrupt all three, just in case they're stuck. // Note: On some platforms, even interrupt might not - // have an effect if the thread is IO blocked. - try { t3.interrupt(); } catch (SecurityException se) {} + // have an effect if the thread is IO blocked. But we + // need to move away from Thread.stop so this is the + // best we can do. + try { + t1.quit(); + t2.quit(); + t3.quit(); + t1.interrupt(); + t2.interrupt(); + t3.interrupt(); + } catch (SecurityException se) {} - // finally, forcibly stop the threads. Yeah, I know. - t1.stop(); - t2.stop(); - t3.stop(); try { t1.join(); } catch (InterruptedException ie) {} try { t2.join(); } catch (InterruptedException ie) {} try { t3.join(); } catch (InterruptedException ie) {}