deivid-rodriguez / byebug

Debugging in Ruby 2
BSD 2-Clause "Simplified" License
3.34k stars 328 forks source link

stepping problems with Fiber/Enumerator #153

Open os97673 opened 9 years ago

os97673 commented 9 years ago

Working on https://youtrack.jetbrains.com/issue/RUBY-16770 (which is a problem in debase) I've noticed that byebug doesn't work well with the following code.

require 'byebug'
triangular_numbers = Enumerator.new do |yielder|
  number = 0
  count = 1
  byebug
  loop do
    number += count
    count += 1
    yielder.yield number
  end
end

byebug
print triangular_numbers.next, " "

5.times do
  # byebug
  print triangular_numbers.next, " "
end

exit

if call "next" when you stopped at byebug inside enumerator I'd expect it will exit the block and stop somewhere (either "5.times" or "print") but actually it works like continue :( Another problem is that if uncomment 3rd "byebug" call and run next there I'd expect byebug will step over "print" and stop at byebug again, but it actually goes into "loop" inside enumarator's block.

It looks like enumarator implementation does something strange (unexpected) in term of stack: it actually completely changes it and thus all byebug's code which relies on stack size works in unexpected way.

As soon as I will fix the crash inside debase I expect the same problems there too, thus I'm filing the ticket to discuss ways to resolve it or decide what we will need from Ruby VM to fix it.

deivid-rodriguez commented 9 years ago

Thanks! Keep me posted on your findings!

os97673 commented 9 years ago

It looks like we see enumerator's implementation details: it replace whole call stack. But all four debugging gems I know (byebug, debugger, debase, and ruby-debug-base) are written in assumption that the only possible stack modifications are push/pop and thus stepping is implemented in term of stack size. I, personally, do not see simple way to handle stack's swap we have in case of enumerators because we need to keep at stack size we had before the next call and also somehow detect that we are in the call (e.g. check if topmost frame has (nil:0) (file:line_no), though this is rather fragile heuristic :(

@deivid-rodriguez do you have any ideas/suggestions how to approach/resolve the problem?

deivid-rodriguez commented 9 years ago

@os97673 You mean instead of pushing a new frame, it replaces the top most frame? Can you point at ruby's source code where that happens?

Sorry I can't be of more help right now, I need to find some time to study both Enumerator and its implementation just to able to answer your question...

os97673 commented 9 years ago

As far as I can see Enumarator#next uses fiber (https://github.com/ruby/ruby/blob/trunk/enumerator.c#L610) which in turn does all dirty hacks (e.g. see https://github.com/ruby/ruby/blob/trunk/cont.c#L1337)

deivid-rodriguez commented 9 years ago

So this is not specific to Enumerator but has to do with Fiber? In that case I guess the next step would be reproducing using Fiber only?

os97673 commented 9 years ago

Here is the test

fiber = Fiber.new do
  number = 0
  count = 1
  loop do
    number += count
    count += 1
    Fiber.yield number
  end
end

puts fiber.resume

5.times do
  puts fiber.resume
end
deivid-rodriguez commented 9 years ago

Nice. Now I need to study fibers instead of enumerators... :smile:

os97673 commented 9 years ago

I've performed more tests and it looks like we receive no events for creation/switching of Fibers :( Imho we need them but it also mean that we will need to keep context for every fiber and (if we decide to support them) we will need to decide what step, step into and step out should do in case of fibers.

What do you think?

deivid-rodriguez commented 9 years ago

I agree... But it might be worth opening an issue in ruby-core maybe?

os97673 commented 9 years ago

As soon as we both agree that it looks like a reasonable API, I will file the issue there and link it to this one.

os97673 commented 9 years ago

https://bugs.ruby-lang.org/issues/11348 created

deivid-rodriguez commented 9 years ago

Thanks @os97673