Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cannot rescue LocalJumpError originated in other thread #4697

Closed
ivoanjo opened this issue Jun 29, 2017 · 5 comments
Closed

Cannot rescue LocalJumpError originated in other thread #4697

ivoanjo opened this issue Jun 29, 2017 · 5 comments
Milestone
JRuby 9.2.0.0

Comments

@ivoanjo
Copy link
Contributor

ivoanjo commented Jun 29, 2017

Environment

Provide at least:

  • JRuby latest master: jruby 9.1.11.0-SNAPSHOT (2.3.3) 2017-06-29 4bc6c50 Java HotSpot(TM) 64-Bit Server VM 25.131-b11 on 1.8.0_131-b11 [linux-x86_64] (Noticed that latest master still reports 9.1.11.0, but this is defnitely latest master)
  • Kernel: Linux maruchan 4.11.0-041100-generic #201705041534 SMP Thu May 4 19:36:05 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
  • Distro: Ubuntu 17.04

Expected Behavior

As a followup of #4686, while I was trying to isolate the problem I also wrote the following rather-synthetic example:

begin
  Thread.new { break :foo }.value
rescue Exception => e
  puts "handled #{e} (#{e.class}) successfully!"
end

puts 'done'

On MRI the output is:

$ ruby -v
ruby 2.4.1p111 (2017-03-22 revision 58053) [x86_64-linux]
$ ruby broken.rb 
handled break from proc-closure (LocalJumpError) successfully!
done

Actual Behavior

Under JRuby, this case does not work properly:

$ jruby -v
jruby 9.1.11.0-SNAPSHOT (2.3.3) 2017-06-29 4bc6c50 Java HotSpot(TM) 64-Bit Server VM 25.131-b11 on 1.8.0_131-b11 [linux-x86_64]
$ jruby broken.rb 
Exception in thread "Ruby-0-Thread-1: broken.rb:2" org.jruby.ir.runtime.IRBreakJump
done

This is lower priority for me than #4686 as I that one bit me on a real-world application and this one was an accidental side discovery.
@headius
Copy link
Member

headius commented Jul 8, 2017

This is fixed by marking all thread procs as escaped, similar to the other fix in #4686. The logic for marking the block escaped worked properly, but the proc duplicated the block and did not preserve its escape state.

@headius
Copy link
Member

headius commented Jul 8, 2017

@ivoanjo Can you add a case for this to ruby/spec, either through our spec/ruby dir or https://github.com/ruby/spec?

@headius headius added this to the JRuby 9.2.0.0 milestone Jul 8, 2017
@headius headius closed this as completed Jul 8, 2017
@ivoanjo ivoanjo mentioned this issue Jul 13, 2017
Merged
@ivoanjo
Copy link
Contributor Author

ivoanjo commented Jul 13, 2017

Thanks for the awesome fix. PR with test for ruby-spec submitted! My first spec PR, hopefully not the last :)

eregon pushed a commit to ruby/spec that referenced this issue Jul 15, 2017
@eregon
Add spec for an invalid break from another thread
837f04b 
This checks that the LocalJumpError is correctly propagated across
threads, see jruby/jruby#4697 .
@eregon
Copy link
Member

eregon commented Jul 27, 2017

Note that the new spec, now in JRuby seems to not pass:

bin/jruby spec/mspec/bin/mspec run -Gfails -Gcritical -Gslow /home/eregon/code/jruby/spec/ruby/language/break_spec.rb
$ /home/eregon/code/jruby/bin/jruby /home/eregon/code/jruby/spec/mspec/bin/mspec-run -Gfails -Gcritical -Gslow /home/eregon/code/jruby/spec/ruby/language/break_spec.rb
jruby 9.1.11.0-SNAPSHOT (2.3.3) 2017-07-27 8246886 OpenJDK 64-Bit Server VM 25.121-b14 on 1.8.0_121-b14 +jit [linux-x86_64]
[/ |                   0%                     | 00:00:00]      0F      0E Exception in thread "Ruby-0-Thread-1: /home/eregon/code/jruby/spec/ruby/language/break_spec.rb:67" org.jruby.ir.runtime.IRBreakJump
                                                                                             
1)
The break statement in a captured block from another thread raises a LocalJumpError when getting the value from another thread FAILED
Expected LocalJumpError but no exception was raised (nil was returned)
/home/eregon/code/jruby/spec/ruby/language/break_spec.rb:73:in `block in (root)'
org/jruby/RubyBasicObject.java:1716:in `instance_eval'
org/jruby/RubyEnumerable.java:1596:in `all?'
org/jruby/RubyFixnum.java:302:in `times'
org/jruby/RubyArray.java:1734:in `each'
org/jruby/RubyArray.java:1734:in `each'
/home/eregon/code/jruby/spec/ruby/language/break_spec.rb:29:in `<main>'
org/jruby/RubyKernel.java:977:in `load'
org/jruby/RubyBasicObject.java:1716:in `instance_eval'
org/jruby/RubyArray.java:1734:in `each'

It looks like the exception is raised on the break and not on .value.
So here is 👍 to a good spec.

@eregon eregon reopened this Jul 27, 2017
@ivoanjo
Copy link
Contributor Author

ivoanjo commented Jul 29, 2017

Wow, weird. I could have sworn that at least when I submitted the spec, 9.1.12.0 was failing but trunk was passing it.

@headius headius closed this as completed in 0697344 Aug 5, 2017
@toy toy mentioned this issue Jan 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 9.2.0.0
Development

No branches or pull requests
3 participants
@headius @eregon @ivoanjo