Skip to content

Remove thread stop#5315

Merged
headius merged 1 commit intomasterfrom
remove_thread_stop
Sep 19, 2018
Merged

Remove thread stop#5315
headius merged 1 commit intomasterfrom
remove_thread_stop

Conversation

@headius
Copy link
Member

@headius headius commented Sep 19, 2018

This PR is to resolve #1183 by replacing the remaining stop calls with interrupt calls.

This may lead to bad behavior in the threads we formerly forced to stop, but it would be better to know that these cases exist than continue to call this bad method.

@headius headius added this to the JRuby 9.2.1.0 milestone Sep 19, 2018
@headius headius requested review from enebo and kares September 19, 2018 18:47
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 headius merged commit ffa6bbb into master Sep 19, 2018
@headius headius deleted the remove_thread_stop branch September 19, 2018 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants