Incompatibility with Zeitwerk

[fxn]

Let me open this issue as a way to exchange impressions about this.

Zeitwerk listens to :class events to load what the project calls "explicit namespaces" (see why at this moment in my talk in RailsConf), but within a Byebug session, these events are not emitted.

Does Byebug need specifically to disable :class events, or could it disable others and preserve this one? If it needs them, could it be a way to make both projects compatible?

Right now, Rails 6 applications with common defaults have this gotcha, for example.

[deivid-rodriguez]

Hi @fxn!

Thanks for zeitwerk, I've been playing around with it, and it's great.

Byebug uses the Tracepoint API very heavily, including :class events. And it does disable them when the debugger stops (for example, when the user hits "continue" and there are no registered breakpoints). But I would expect that only the traces registered by byebug itself are disabled, not all the traces registered by other libraries... :thinking:

[fxn]

But I would expect that only the traces registered by byebug itself are disabled, not all the traces registered by other libraries

Exactly, I was wondering that too.

I am trying to reproduce, and apparently the description is not accurate. The enabled? predicate returns true in the tracer, but the block is not called. I have uploaded a demo app to show the issue here.

If you clone and bundle install, then a session to reproduce may be like this:

$ bin/rails r 'User.debug_me'
Return value is: nil

[1, 5] in /Users/fxn/tmp/ByebugZeitwerkDemo/app/models/user.rb
   1: class User
   2:   def self.debug_me
   3:     byebug
=> 4:   end
   5: end
(byebug) Zeitwerk::ExplicitNamespace.tracer.enabled?
true
(byebug) Hotel
*** NameError Exception: uninitialized constant Hotel::Pricing

nil

Hotel is an explicit namespace that should autoload just fine:

$ bin/rails r 'p Hotel'
Hotel

Mystery!

If you'd like to modify the block to debug, the tracer is defined here.

[deivid-rodriguez]

Thanks for the reproduction steps, I'll see if I can figure it out!

[fxn]

Ah! I forgot @palkan investigated this and left some comments in https://github.com/fxn/zeitwerk/issues/31#issuecomment-479996317.

[fxn]

This is a minimal reproduction that does not depend on Rails or Zeitwerk:

→ tmp$ cat bar.rb
class Bar
end

→ tmp$ cat foo.rb
TracePoint.new(:class) { |tp| p :CALLED }.enable
autoload :Bar, "bar"
byebug
1

→ tmp$ ruby -I. -rbyebug foo.rb
:CALLED
:CALLED
:CALLED
<many of these>

[1, 4] in /Users/fxn/tmp/foo.rb
   1: TracePoint.new(:class) { |tp| p :CALLED }.enable
   2: autoload :Bar, "bar"
   3: byebug
=> 4: 1
(byebug) Bar
Bar

We should see :CALLED too.

[deivid-rodriguez]

Nice. Yeah, after reading the message you linked to I understood the problem, and I think it should be fixable inside byebug. I'll prioritize this.

[deivid-rodriguez]

Hei @fxn!

So I had a closer look at this yesterday and managed to narrow down the problem. In principle, I think the solution belongs in ruby-core :disappointed:.

I think the reason why this doesn't work is the same reason why the following script

TracePoint.trace(:class) do |_tp|
  puts "CLASS"
end

# trigger a class event
binding.eval("class Hola; end")

TracePoint.trace(:line) do |tp|
  puts "LINE"

  # trigger a class event, ignored :(
  tp.binding.eval("class Adios; end")
end

# trigger a line event
1 + 1

prints

CLASS 
LINE

instead of

CLASS
LINE
CLASS

The problem is that when we are in the debugger prompt, we're actually in the middle of a TracePoint event handler (normally, a :line event). And the TracePoint API forbids events to be triggered during handler's code. It makes sense to do so, otherwise the previous script would loop forever, since the handler code for line events would trigger line events itself.

See https://github.com/ruby/ruby/blob/02880d1f4a9ebd1c0a807376fcb25ccd908334b4/vm_trace.c#L383-L400.

A potential solution would be to allow some level of reentrancy by keeping a stack instead of a single flag, and check that the current event type is not already in the stack. That would allow call events to be triggered from line events, but not line events to be triggered from line events, for example.

I'll bring this to ruby-core.

[zreisman]

Just wondering if there is any intention to support this? Moving project to 'break' at this point. Wondering if you've considered adopting whatever approach they are using?

[deivid-rodriguez]

Sorry, I haven't had any chance to work on this, but I'd love to support it of course! Contributions welcome!

[amfarrell]

@zreisman could you paste a snippet of the code you used to get break to work? It would help those folks like me who've run into this problem to get debugging to work again.

[eregon]

Link to the Ruby issue about this: https://bugs.ruby-lang.org/issues/15912

[eregon]

Does anyone know what's the trick that break uses to avoid the nested TracePoint problem? It seems both byebug and break use TracePoint, so not sure what's the difference. Also: https://github.com/rails/rails/issues/42264

[fxn]

@eregon if my memory does not fail me, it has to do with running things in different fibers /cc @gsamokovarov.

[gsamokovarov]

break's idea is to run the debugging input code in a different fiber so it can be executed a bit later and outside the current tracepoint context.

https://github.com/gsamokovarov/break/blob/950b3ba137ba65fc2935bc83cea270c887c383b7/lib/break/commands/tracepoint_command.rb#L16-L23

https://github.com/gsamokovarov/break/blob/950b3ba137ba65fc2935bc83cea270c887c383b7/lib/break/commands/tracepoint_command.rb#L41-L45

This approach has its drawbacks as well. You can't debug code in a fiber initiated by another thread. The nice thing about the approach is that it can be done in another layer of the debugger, say closer to the REPL itself rather than the actual internals.

[d4rky-pl]

Seems that TracePoint.allow_reentry is now a thing, would that make solving this issue possible? @deivid-rodriguez

[brasic]

@d4rky-pl as far as I’m aware, yes. the same issue affected the debug gem and was fixed by the use of TracePoint.allow_reentry. See https://github.com/ruby/debug/issues/408 and https://bugs.ruby-lang.org/issues/15912.

[d4rky-pl]

@brasic unfortunately the TracePoint part in byebug is implemented in C which is way above my pay grade so I can't contribute 🥲

Here's hoping someone will be interested in picking this up. I really love byebug and pry and can't imagine working without them. It's probably one of the most important libraries in my developer career 😅

[marshall-lee]

Hey guys I came up with a solution. Your reviews & testing are welcome https://github.com/deivid-rodriguez/byebug/pull/847

@deivid-rodriguez please rake docker:build_all from my branch to update container images for testing. I added Ruby 3 boxes and bumped the ruby versions in 2.x containers.