Bug #4285
closedRuby don't have asynchrounous exception safe syntax and It should have.
Description
=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
Files
Updated by mame (Yusuke Endoh) almost 14 years ago
=begin
Hi,
2011/1/17 Motohiro KOSAKI redmine@ruby-lang.org:
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 mame@tsg.ne.jp
=end
Updated by mwaechter (Matthias Wächter) almost 14 years ago
- File timeout.rb.diff timeout.rb.diff added
=begin
Why not use a mutex? Too big a performance hit?
=end
Updated by mwaechter (Matthias Wächter) almost 14 years ago
- File timeout.rb.diff timeout.rb.diff added
=begin
Why not use a mutex? Too big a performance hit?
=end
Updated by kosaki (Motohiro KOSAKI) almost 14 years ago
=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
Updated by mame (Yusuke Endoh) almost 14 years ago
=begin
Hi,
2011/1/18 Motohiro KOSAKI redmine@ruby-lang.org:
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
endtimeout (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
endexception 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
endexception may be raised¶
end
t.raise -
async exception is blocked by default in rescue clauses and
ensure clausest = Thread.new do
begin
# exception may be raised
ensure
# exception may NOT be raised
allow_async_exc do
# exception may BE raised
end
endexception may be raised¶
end
t.raise
I remember that we discuss this topic with akr informally, but I forgot
the conclusion...
--
Yusuke Endoh mame@tsg.ne.jp
=end
Updated by zimbatm (zimba tm) almost 14 years ago
=begin
2011/1/18 Yusuke ENDOH mame@tsg.ne.jp:
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
Updated by zimbatm (zimba tm) almost 14 years ago
=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) jonas@pfenniger.name:
2011/1/18 Yusuke ENDOH mame@tsg.ne.jp:
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
Updated by headius (Charles Nutter) almost 14 years ago
=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
Updated by mame (Yusuke Endoh) almost 14 years ago
=begin
Hi,
2011/1/19 Charles Nutter redmine@ruby-lang.org:
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 mame@tsg.ne.jp
=end
Updated by usa (Usaku NAKAMURA) almost 14 years ago
- Status changed from Open to Assigned
=begin
=end
Updated by ko1 (Koichi Sasada) over 12 years ago
- Description updated (diff)
- Assignee changed from matz (Yukihiro Matsumoto) to ko1 (Koichi Sasada)
Updated by ko1 (Koichi Sasada) about 12 years ago
I don't follow all of discussion about it, this is solved by [ruby-trunk - Feature #6762]?
Updated by ko1 (Koichi Sasada) almost 12 years ago
- Status changed from Assigned to Feedback
- Target version changed from 2.0.0 to 2.6
Updated by ko1 (Koichi Sasada) almost 12 years ago
- 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.