Bug #4285: Ruby don't have asynchrounous exception safe syntax and It should have.

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.6
• ruby -v: ruby 1.9.3dev (2010-12-22 trunk 30291) [x86_64-linux]

[kosaki (Motohiro KOSAKI)]

=begin
This issue was discovered during [Bug#4266] discussion.
Current timeout is racy.

Now, timeout module has following code.

def timeout()
begin
x = Thread.current
y = Thread.start {
begin
sleep sec
rescue => e
x.raise e
else
x.raise exception, "execution expired" if x.alive?
end
}
return yield(sec)
rescue exception => e
raise Error, e.message, e.backtrace
ensure
if y and y.alive?
y.kill
y.join # make sure y is dead.
end
end
end

Unfortunatelly,

y = Thread.start {}

is not an atomic operation. Then, A following race can occur.

CPU0(thread x) CPU1(thread y) remark

enter begin block
[thread construct] but no assign y yet
sleep sec
wakeup from sleep
x.raise
if y return false. (see above)

Therefore, CPU0 don't call y.join and leak y's thread resource. C# have solved
this two-step-construction vs asynchrounous exception race by RAII.
(Plus, C#'s finally block is async exception safe automatically)

But unfortunately, Ruby don't have such language feature. So, We can't write
async-exception-safe code. One of solution is to move timeout module from ruby code
into c code as JRuby does. But I don't think timeout is only asynchrounos exception user.
we also have Interrupt class (for Ctrl-C) and I think we need to allow to write async
exception safe code by ruby.

So, My proposal is,
o for 1.9.x
=> Reimplement timeout.rb by C (as JRuby)

o for 2.0
=> Aim new feature for writing async exception safe code.

Or, Am I missing something?
=end

[mame (Yusuke Endoh)]

=begin
Hi,

2011/1/17 Motohiro KOSAKI [e-mail]:

 CPU0(thread x)                     CPU1(thread y)         remark

 enter begin block
 [thread construct]                                        but no assign y yet
                                    sleep sec
                                    wakeup from sleep
                                    x.raise
 if y                                                      return false. (see above)

Therefore, CPU0 don't call y.join and leak y's thread resource.

What's the resource?
I think that the thread will terminate automatically even if it is
not join'ed.

But unfortunately, Ruby don't have such language feature. So, We can't write
async-exception-safe code. One of solution is to move timeout module from ruby code
into c code as JRuby does. But I don't think timeout is only asynchrounos exception user.
we also have Interrupt class (for Ctrl-C) and I think we need to allow to write async
exception safe code by ruby.

Dr. akr knows a lot about that subject.

--
Yusuke Endoh [e-mail]

=end

[mwaechter (Matthias Wächter)]

• File timeout.rb.diff timeout.rb.diff added

=begin
Why not use a mutex? Too big a performance hit?

=end

[mwaechter (Matthias Wächter)]

• File timeout.rb.diff timeout.rb.diff added

=begin
Why not use a mutex? Too big a performance hit?

=end

[kosaki (Motohiro KOSAKI)]

=begin
Endoh-san, Grr, my fault. I thought Thread.join call pthread_join, but it doesn't. So right you are, thread variable leak doesn't cause serious error.
But generically, Asynchrounous exception during OS resource handler class construction is still dangerous. new example is here.

def foo
begin
f = open("example-file")
ensure
f.close
end

timeout (1) {
foo
}

if assignment of f was lost, we have no way to close f.
So, I agree that reimplementation by C doesn't solve anything. because source of problem is not in timeout.rb, is in asynchrounous exception semantics!

So, we don't need anything at 1.9.x timeframe. but I still propose language enhancemnet for any async exception for 2.0.

Mathias, Good question! Your proposal solve almost async execption problem. But please imazine nested timeout (maybe it can occur when
a library use timeout internally) or Ctrl-C. In other words, mutex is effective only when exception source is recognized. But generically, async exception source are not.

=end

[mame (Yusuke Endoh)]

=begin
Hi,

2011/1/18 Motohiro KOSAKI [e-mail]:

Endoh-san, Grr, my fault. I thought Thread.join call pthread_join, but it doesn't.

I thought so :-)

But generically, Asynchrounous exception during OS resource handler class construction is still dangerous. new example is here.

def foo
begin
f = open("example-file")
ensure
f.close
end

timeout (1) {
foo
}

if assignment of f was lost, we have no way to close f.

Yes. Surely, the current Thread#raise is dangerous.

I still propose language enhancemnet for any async exception for 2.0.

Basically agreed, but what is needed actually?
I guess that we need these three features, including a semantics
change.

• a feature to block async exception

  t = Thread.new do

  exception may be raised

  block_async_exc do
  # exception may NOT be raised
  end

  exception may be raised

  end
  t.raise

• a feature to allow async exception

  t = Thread.new do

  exception may be raised

  block_async_exc do
  # exception may NOT be raised
  allow_async_exc do
  # exception may BE raised
  end
  # exception may NOT be raised
  end

  exception may be raised

  end
  t.raise

• async exception is blocked by default in rescue clauses and
  ensure clauses

  t = Thread.new do
  begin
  # exception may be raised
  ensure
  # exception may NOT be raised
  allow_async_exc do
  # exception may BE raised
  end
  end

  exception may be raised

  end
  t.raise

I remember that we discuss this topic with akr informally, but I forgot
the conclusion...

--
Yusuke Endoh [e-mail]

=end

[zimbatm (zimba tm)]

=begin
2011/1/18 Yusuke ENDOH [e-mail]:

Basically agreed, but what is needed actually?

I think that ensure blocks should protect us from async exceptions.
They're here to make sure some part of a code is executed, that should
also be the case for async exceptions.

=end

[zimbatm (zimba tm)]

=begin
Actually, protecting from async exceptions in the ensure block would
be mostly backward-compatible. It means that while a thread is in that
part of the code, no other thread would be running. Only code that
will break, would be code that calls timeout, or does big
calculations, while being in that block.

Another idea: most timeouts are on IO. If IO#gets for example issues
the timeout exception, then it's easy to catch it, you just need to
wrap it with a begin; rescue TimeoutError; end block. It's the same
for the other IOs. Since the timeout happens in C-land, there is no
need to raise that async exception anymore. If that works out well,
then the timeout.rb feature can also be removed. This would require to
set a default timeout on IO and also make it overridable per instance,
but you get the idea.

2011/1/18 Jonas Pfenniger (zimbatm) [e-mail]:

2011/1/18 Yusuke ENDOH [e-mail]:

Basically agreed, but what is needed actually?

I think that ensure blocks should protect us from async exceptions.
They're here to make sure some part of a code is executed, that should
also be the case for async exceptions.

=end

[headius (Charles Nutter)]

=begin
It is not possible to make cross-thread exception-raising safe with current Ruby semantics.

See the discussion on my blog post here: http://blog.headius.com/2008/02/rubys-threadraise-threadkill-timeoutrb.html

No matter how many locks you use, there's always potential for an arbitrary thread to receive an exception or kill request while executing or immediately before "ensure" code. This breaks the contract of ensure. Ergo, asynchronous exceptions (exceptions thrown from one thread into another) and killable threads are inherently not possible to make safe if you want to obey the contract of "ensure".

JRuby is affected by this as well, since we have to emulate MRI's asynchonous exception throwing features.

There is a way to change Ruby semantics to make asynchronous exceptions and "kill" safe: do not allow them to terminate a thread running an ensure block until that block has completed. However, I think this is mostly useless; a top-level thread that fires logic in its ensure would prevent the entire thread from being killable or raiseable.

Ultimately, both of these features should be removed. And I know that's not going to happen, because everyone uses them :)
=end

[mame (Yusuke Endoh)]

=begin
Hi,

2011/1/19 Charles Nutter [e-mail]:

I tried to patch this many times, and it's very difficult. The problem here is that kill is inherently broken in the presence of an ensure block. I've discussed this on ruby-core emails in the past and on my blog here: http://blog.headius.com/2008/02/rubys-threadraise-threadkill-timeoutrb.html

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.

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.

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.

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.

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.

--
Yusuke Endoh [e-mail]

=end

[usa (Usaku NAKAMURA)]

• Status changed from Open to Assigned

=begin

=end

[ko1 (Koichi Sasada)]

• Description updated (diff)
• Assignee changed from matz (Yukihiro Matsumoto) to ko1 (Koichi Sasada)

[ko1 (Koichi Sasada)]

I don't follow all of discussion about it, this is solved by [ruby-trunk - Feature #6762]?

[ko1 (Koichi Sasada)]

• Status changed from Assigned to Feedback
• Target version changed from 2.0.0 to 2.6

[ko1 (Koichi Sasada)]

• Status changed from Feedback to Closed

This issue was solved with changeset r38046.
Koichi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• thread.c: rename Thread.control_interrupt
  to Thread.async_interrupt_timing.
  The option name :never' is also changed to :defer'.
  [ruby-core:50375] [ruby-trunk - Feature #6762]
• thread.c: remove Thread.check_interrupt.
  This method is difficult to understand by name.
• thraed.c: add Thread.async_interrupted?.
  This method check any defered async interrupts.
• test/ruby/test_thread.rb: change tests for above.