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.

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

LocalJumpError raised on break from custom Enumerator enumeration #1270

Closed
tdg5 opened this Issue Nov 26, 2013 · 9 comments

Comments

Projects
None yet
5 participants
@tdg5
Copy link

tdg5 commented Nov 26, 2013

I have little to no experience with JRuby, so it's possible I'm doing this wrong, but while I was working to make a library I'm developing JRuby compatible, I ran into some weirdness with the Enumerator class.

Long story short, breaking from a custom Enumerator raises "LocalJumpError: break from proc-closure". This same behavior is not observed when breaking from an enumerator obtained via Range#to_enum. I understand this may be related to how the enumerator is created by Range, so I would be open to suggestions on how I might refactor the code below to support break.

I looked through the specs and came across TestIterator#test_ljump which made it seem that a LJE when breaking from a block may be the desired behavior. However, the same spec also makes it seem like a break from a lambda should not raise a LJE. To this end I also tried building an Enumerator using a lambda to determine if that fixed the issue, however a LJE is still raised, though with a different message.

Here is some code I wrote up to demonstrate this issue. This same code does not raise an error when run against MRI 1.9.3 or 2.0.0. However for JRuby, I found this code raises a LJE in both 1.9 and 2.0 mode of JRuby 1.7.6 and 1.7.8. I'm running Ubuntu 12.04 and I've included full ruby -v lines at the end of the issue.

unbreakable.rb:

class Unbreakable

  def range_enumerator
    (0..10).to_enum
  end


  def block_enumerator
    Enumerator.new do |yielder|
      10.upto(20) do |i|
        yielder << i
      end
    end
  end


  def lambda_enumerator
    Enumerator.new(&(lambda do |yielder|
      20.upto(30) do |i|
        yielder << i
      end
    end))
  end

end

def main
  unbreakable = Unbreakable.new

  puts "Range#to_enum class: #{unbreakable.range_enumerator.class}"
  puts "Enumerator instance w/ block class: #{unbreakable.block_enumerator.class}"
  puts "Enumerator instance w/ lambda class: #{unbreakable.lambda_enumerator.class}"

  count = 0
  unbreakable.range_enumerator.each do |x|
    puts x
    break if 5 == count += 1
  end

  count = 0
  begin
    unbreakable.block_enumerator.each do |x|
      puts x
      break if 5 == count += 1
    end
  rescue => e
    puts "Fail: #{e.inspect}"
  end

  count = 0
  begin
    unbreakable.lambda_enumerator.each do |x|
      puts x
      break if 5 == count += 1
    end
  rescue => e
    puts "Fail: #{e.inspect}"
  end
end

main

MRI output:

$ ruby unbreakable.rb
Range#to_enum class: Enumerator
Enumerator instance w/ block class: Enumerator
Enumerator instance w/ lambda class: Enumerator
0
1
2
3
4
10
11
12
13
14
20
21
22
23
24

JRuby output:

Range#to_enum class: Enumerator
Enumerator instance w/ block class: Enumerator
Enumerator instance w/ lambda class: Enumerator
0
1
2
3
4
10
11
12
13
14
Fail: #<LocalJumpError: break from proc-closure>
20
21
22
23
24
Fail: #<LocalJumpError: unexpected break>

My full ruby -v are:

jruby 1.7.6 (1.9.3p392) 2013-10-22 6004147 on OpenJDK 64-Bit Server VM 1.7.0_25-b30 [linux-amd64]
jruby 1.7.8 (1.9.3p392) 2013-11-14 0ce429e on OpenJDK 64-Bit Server VM 1.7.0_25-b30 [linux-amd64]

Please let me know if I can answer any questions or provide any additional information!

@headius

This comment has been minimized.

Copy link
Member

headius commented Nov 27, 2013

This is indeed a bug! It won't make it into 1.7.9 but I will mark it for 1.7.10 and we should see what we can do with it.

Work noting is that it fails the same with 1.7.4, which had our first Enumerator implementation, so it's not a regression in 1.7.5 and later (which have a new Enumerator impl).

Definitely should be fixed. Thank you for the reproduction!

@headius headius modified the milestones: JRuby 9000, JRuby 1.7.10 Feb 21, 2014

@f3nry

This comment has been minimized.

Copy link

f3nry commented Jun 26, 2014

We ran into this issue today on a project. We'd love to have this fixed in the next release! 😊

@jcogilvie

This comment has been minimized.

Copy link

jcogilvie commented Feb 27, 2015

Thirded. Ran headlong into this today after porting a bunch of code to C# to Ruby. We'll have to ditch ruby altogether if it can't be fixed. =(

@enebo enebo modified the milestones: JRuby 1.7.20, JRuby 9.0.0.0 Mar 2, 2015

@headius

This comment has been minimized.

Copy link
Member

headius commented Mar 2, 2015

Looking into this today.

@headius

This comment has been minimized.

Copy link
Member

headius commented Mar 2, 2015

@svtdragon @f3nry If you have a more narrow case for your examples, I would appreciate it. The LongJumpError can happen for many reasons, and I'd like to understand if your cases are the same or different.

@jcogilvie

This comment has been minimized.

Copy link

jcogilvie commented Mar 2, 2015

I've managed to stop getting the error with the help of a ruby expert--I think I was just doing something stupid, but trying to get the debugger working while jumping back and forth between ruby and java is hard, so it's hard for me to say what specifically.

@headius

This comment has been minimized.

Copy link
Member

headius commented Mar 2, 2015

@svtdragon Ok, thanks for the follow-up. I think I have a fix sorted out for the reported issue.

headius added a commit that referenced this issue Mar 2, 2015

Only consider breaks that originated from this proc's frame.
The old logic here was turning all breaks that pass through an
escaped proc into LongJumpError, even if they originated in a
different frame/proc that could still validly handle the break.
This commit modifies the logic to ignore the exception altogether
(re-throwing it) if it is not associated with this proc/frame (by
checking jumpTarget).

Fixes #1270

headius added a commit that referenced this issue Mar 2, 2015

Add spec for GH-1270.
Note that there's an unfixable (in 1.7) edge case described and
disabled in the spec. I will apply the unmodified case to master,
which handles it ok.

@headius headius closed this in e61b737 Mar 2, 2015

headius added a commit that referenced this issue Mar 2, 2015

@headius

This comment has been minimized.

Copy link
Member

headius commented Mar 2, 2015

Ok, we have fixes for this on 1.7 and master.

There's one caveat for 1.7: if an "escaped" proc (like the Enumerator block in the original example) and a separate block's break appear within the same method body (as in the one-liner version of the spec added to master), JRuby 1.7 can't tell that the break is intended for the outer block rather than the inner one. In that case, it will still prematurely turn the jump into a LocalJumpError. We may look at a fix for this in the future, but it's an obscure and uncommon case.

JRuby master (9k) is able to handle all cases correctly.

@headius headius self-assigned this Mar 2, 2015

enebo added a commit that referenced this issue Mar 20, 2015

Revert "Only consider break jumps actually originating from this proc."
This reverts commit fad37ea.

commit 85ffd61 eliminates the need for this extra logic.

Fixes #1270
@enebo

This comment has been minimized.

Copy link
Member

enebo commented Mar 20, 2015

#2736 had a similar problem but it solves both issues on master. Basically, marking generator as a LAMBDA was confusing our nonlocal flow logic thinking we had a hard (method) scope so we would dump out in generator instead of the intended scope. Changed to a PROC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.