Make Thread#raise not raise $! #1942

Merged
merged 3 commits into from Oct 29, 2012

Projects

None yet

3 participants

@ryoqun
Rubinius member

Hahaha, what a corner case I'm fixing... Dunno it's worthwhile...

Also, is there any better way to write these specs?

@brixen brixen commented on an outdated diff Oct 4, 2012
spec/ruby/core/thread/raise_spec.rb
@@ -133,8 +151,79 @@
end
end
+ ruby_version_is "" ... "1.9" do
+ it "raises a RuntimeError when called with no arguments inside rescue" do
@brixen
brixen Oct 4, 2012

it "raises a ZeroDivisionError..." no?

@brixen brixen commented on an outdated diff Oct 4, 2012
spec/ruby/core/thread/raise_spec.rb
end
describe "Thread#raise on same thread" do
it_behaves_like :kernel_raise, :raise, Thread.current
+
+ ruby_version_is "" ... "1.9" do
+ it "raises a RuntimeError when called with no arguments inside rescue" do
@brixen
brixen Oct 4, 2012

ZeroDivisionError?

@brixen brixen commented on an outdated diff Oct 4, 2012
spec/ruby/core/thread/raise_spec.rb
@@ -133,8 +151,79 @@
end
end
+ ruby_version_is "" ... "1.9" do
+ it "raises a RuntimeError when called with no arguments inside rescue" do
+ raised = false
+ t = Thread.new do
+ begin
+ 1/0
+ rescue ZeroDivisionError
+ raised = true
+ loop { }
+ end
+ end
+ begin
+ raise RangeError
+ rescue
+ Thread.pass until raised || !t.alive?
@brixen
brixen Oct 4, 2012

Are we sure that t.alive? will return true from the very first time this could get executed (ie could t.alive? return false before the thread runs)? In other words, is this guaranteed race-free given MRI's very fuzzy thread semantics? I prefer to use explicit coordination / synchronization rather than depending on Thread state predicates.

ryoqun added some commits Oct 4, 2012
@ryoqun ryoqun Add more specs for Thread#raise with no arg inside rescue 788a5b0
@ryoqun ryoqun Make Thread#raise not raise $!
There is 1.8/1.9-specific behaviors when calling Thread#raise with no arguments
inside a rescue clause and the called thread is also inside a rescue clause.

First of all, Thread#raise doesn't use the exception in the calling thread. In
other words, Thread#raise doesn't use $!, only Kernel#raise does.

In 1.8, the raised exception is the exception in the called thread.

In 1.9, it is always a RuntimeError.
c916039
@ryoqun ryoqun Remove passing tags ac94dc2
@ryoqun
Rubinius member

@brixen Thanks for reviewing! First, I fixed blatant wrong spec descriptions (RuntimeError => ZeroDivisionError). Second, I closely looked the implementation of specs and noticed that it's not needed to check Thread state predicates. So, I remove them.

@dbussink dbussink merged commit 5c9506b into rubinius:master Oct 29, 2012

1 check failed

Details default The Travis build failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment