Skip to content

Commit

Permalink
Remove remaining JDK Thread#stop calls for #1183.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
headius committed Sep 19, 2018
1 parent ae777f0 commit 79b6b33
Showing 1 changed file with 14 additions and 10 deletions.
24 changes: 14 additions & 10 deletions core/src/main/java/org/jruby/util/ShellLauncher.java
Expand Up @@ -1473,7 +1473,7 @@ public void quit() {
synchronized (waitLock) {
waitLock.notify();
}
stop();
interrupt();
}
}

Expand Down Expand Up @@ -1523,9 +1523,8 @@ public void run() {
}
}
public void quit() {
interrupt();
this.quit = true;
stop();
interrupt();
}
}

Expand Down Expand Up @@ -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) {}
Expand Down

0 comments on commit 79b6b33

Please sign in to comment.