[ruby-core:34589] Re: [Ruby 1.9-Bug#4283] Timeout.timeout may cause application exit unintetionally

On Tue, Jan 18, 2011 at 10:00 PM, Yusuke ENDOH <[e-mail]> wrote:
> This is not just a problem of Thread#raise.  Asynchronous signals
> (such as Interrupt caused by Ctrl+C) have the same problem.
> Of course, we cannot remove Ctrl+C.

The JVM approach may be of interest here. JVMs spin up a
signal-handling thread independent of the main thread. Signal-handlers
are then triggered on this thread rather than on whatever thread
happens to be running at any given time. Registering a signal handler
gives it to that thread to be run when the signal fires (only via
unofficial APIs, but the JVMs all work pretty similarly).

Ctrl+C is, by default, handled as a VM-level shutdown hook. All
threads are terminated immediately, without any code continuing to
run. Contrast this with MRI, where Ctrl+C triggers an Interrupt
exception to be raised:

~/projects/jruby ➔ ruby -e 'begin; sleep; ensure; puts "I should be dead!"; end'
^CI should be dead!
-e:1:in `sleep': Interrupt
	from -e:1

This decision means that Ctrl+C triggers finally blocks to run, which
can then continue to block:

~/projects/jruby ➔ ruby -e 'begin; sleep; rescue Interrupt; puts "I do
not want to die!"; begin; sleep; ensure; puts "Ok fine, I will die.";
end; end'
^CI do not want to die!
^COk fine, I will die.
-e:1:in `sleep': Interrupt
	from -e:1

This of course has identical problems to raising in another thread
that might be running ensure logic, but Ctrl+C is a special case; it's
probably valid to do almost anything, including a hard shutdown
without running ensures at all.

I'm not familiar with how userland signal-handlers fire, but if they
were guaranteed to run on a separate thread, the problems of
interrupting a thread in an ensure would not be a problem.

> Thus, to address this problem faithfully, we should provide a
> mechanism to safely handle asynchronous exceptions.  Lobbying to
> eliminate only Thread#raise (and #kill) is not facing the reality.

Eliminating them (and fixing signal handling) eliminates the problem
:) But I recognize that they're never going to go away. I've tried for
years to make it happen.

> Fortunately, there are some ancient wisdoms:
>
>  - "cancellation points" of pthread
>    http://www.kernel.org/doc/man-pages/online/pages/man7/pthreads.7.html
>
>  - Asynchronous Exceptions in Haskell
>    http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.26.1040
>
> These two are based on a very similar idea: providing a feature
> to control whether asynchronous exceptions may be raised or may
> not.  In fact, the latter is referred in comments of your blog
> article.  But it was rejected as:
>
>> there are a lot of additional problems when implementing it in an environment that isn't as functionally pure as Haskell
>
> I guess that this is misinterpretation.  It is very similar to the
> former (cancellation points), and can be implemented even in
> imperative programming language, as pthread does.  I don't know
> that they are compatible with Java (and/or JRuby) threads, though.

JRuby implements Thread#raise and #kill using exactly this mechanism.
On boundaries roughly equivalent to MRI 1.8's thread context switches,
we ping thread state to see if a kill or raise event has been sent to
the thread. If so, we propagate that event. I documented these on a
now-shutdown wiki, but you can figure them out by looking for places
1.8 checked "thread variables" to see if it should attempt a context
switch. They are places like literal nil, newlines, every 256 calls
(we do it every 1024, I believe), and so on.

In this way, we're able to implement #raise and #kill without using
unsafe JVM thread operations like java.lang.Thread.stop and
java.lang.Thread.stop(Throwable).

I don't see that other concurrent-threaded Ruby implementations will
have any choice but to do it the same way.

The reason we still have the problems of asynchronous raising and
killing is because the cancellation points are poorly defined. Since
we based ours on MRI thread context-switch boundaries, there's simply
too many of them (which is why we've removed a few and increased the
call-count event ping). They also do not reflect the criticality of
certain sections of code, like ensure bodies and code downstream from
them.

As I said in my previous comment...it's not possible to make
asynchronous exceptions safe with the above semantics (the semantics
of Ruby thread context-switching, currently) intact. It may be
possible to make asynchronous exceptions *safer* if we more clearly
define these cancellation points and disable cancellation within
certain contexts.

I must reiterate, however, that this is only a band-aid. Because Ruby
is not "functionally pure", a top-level method might wrap everything
in an ensure block. Should the entire application disable kill/raise?
This would make many applications' threads totally immune to
cancellation. Should leaving an ensure block by making a call
re-enable cancellation? This hardly seems practical, since nearly
everything in Ruby is a call...including the code the user wanted to
"ensure" would run.

I don't want to be difficult. I would very much like to see a more
formal thread event model put in place to "help" this issue, since it
would make our job on JRuby (with concurrent threads) much easier.

> On a separate note, I'm not against deprecating Thread#raise.
> It is indeed too difficult to use correctly.  Just eliminating it,
> however, is not enough.

But it would be so nice for those of us implementing Ruby with real
concurrent threads :) I've spent a lot of time thinking about these
issues, and they're really hard to solve.

I do hope that any ideas going forward will keep concurrent-threaded
implementations like JRuby in mind. We have worked very hard to
emulate Ruby, and it would be very unfortunate if this work went in a
direction impossible to implement safely or efficiently with
concurrent threads.

>> the child thread may still wake up between the end of the user-defined block and the call to kill
>
>
> Yes, it may occur.  But does it cause any actual problem in the
> case of timeout.rb?  Kosaki's patch seems to me good.

Yes, it should fix this particular problem. And it works because in
this case, we actually *want* the asynchronous raise to interrupt the
ensure block. My comments about the difficulty of fixing it were more
appropriate for your other bug on fixing asynchronous exceptions.

This is also similar to what we do internally in JRuby, but we disable
Ruby's normal #raise cancellation points entirely for one of the two
threads. The patch would not guarantee safety on JRuby because thread
context switches can occur at any time, which means the two threads
could attempt to kill each other at exactly the same time. But it
should work on non-concurrent implementations, I think.

- Charlie