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

Allow using binding.bp to start a debug session #65

Closed
st0012 opened this issue Jun 4, 2021 · 33 comments
Closed

Allow using binding.bp to start a debug session #65

st0012 opened this issue Jun 4, 2021 · 33 comments

Comments

@st0012
Copy link
Member

st0012 commented Jun 4, 2021

As stated in the readme, the correct way to start a debug session in the middle of a program is:

require "debug"
DEBUGGER__.console

And then the user can freely use binding.bp to insert new breakpoints.

This is a fine workflow, but I think we can make it more simpler. Since we already have the binding.bp method, which just looks like byebug and binding.pry, why don't we make it work like them?

require "pry"
binding.pry

require "byebug"
byebug

require "debug"
binding.bp

Then the users will have one less thing to learn 🙂

(I have to admit that it took me a while to remember to call DEBUGGER__.console, which I still forget from time to time 😂)

@ko1
Copy link
Collaborator

ko1 commented Jun 5, 2021

Yes, it is planned idea.

The drawback is, explicit debug start will track the program execution:

  • Thread creation
  • Code loading including eval (store loaded Ruby code to show)
  • And more (not yet)

In many cases it is not a problem.


Terminology:

  • Load debug functionality: Loading the debug classes/modules/methods
    • require 'debug'
    • require 'debug/session'
  • Start debug: load debug functionality and start tracking the information for debugging. Also load .rdbgrc for configuration.
    • require 'debug/run'
    • DEBUGGER__.console
  • Stop in a debugger context:
    • breakpoint
    • binding.bp
    • Ctrl-C on (remote) console

Maybe this proposal is, if binding.bp is called without start debugging, start debugging and stop at this location.

@ko1
Copy link
Collaborator

ko1 commented Jun 5, 2021

Another possibility is, require 'debug' start debgging but don't stop the program (== RUBY_DEBUG_NONSTOP mode).
The drawback is, if user doesn't want to collect information, the interpreter starts collection by adding gem 'debug' in Gemfile.

Maybe explicit require 'debug' can introduce it because programmer want to use it.
But gem 'debug' in Gemfile it seems not explicit to introduce additional overhead.

@ko1
Copy link
Collaborator

ko1 commented Jun 5, 2021

In a future, like binding.irb, I want to introduce it as standard. But there is an issue describe above.

@st0012
Copy link
Member Author

st0012 commented Jun 5, 2021

Maybe this proposal is, if binding.bp is called without start debugging, start debugging and stop at this location.

That's what I have in mind too. Should I open a PR for it?

@ko1
Copy link
Collaborator

ko1 commented Jun 5, 2021

I showed some points, and we need to choose one.

@st0012
Copy link
Member Author

st0012 commented Jun 6, 2021

Ah sorry I didn't express that clearly: I took that line as

if binding.bp is called without start debugging, start "debugging session" and stop at this location., which is like

# move this out from Session.initialize_session
Binding.module_eval do
  def bp command: nil
    session_initialized = ::DEBUGGER__.const_get(:SESSION)

    ::DEBUGGER__.console unless session_initialized

    if command
      cmds = command.split(";;")
      SESSION.add_initial_commands cmds
    end

    ::DEBUGGER__.add_line_breakpoint __FILE__, __LINE__ + 1, oneshot: true if session_initialized
    true
  end
end

@ko1
Copy link
Collaborator

ko1 commented Jun 7, 2021

Yes. I know. And I showed some drawback #65 (comment)

@st0012
Copy link
Member Author

st0012 commented Jun 7, 2021

let me summarize our options here:

  1. start the debugging session within binding.bp call if it's not started yet.

if binding.bp is called without start debugging, start "debugging session" and stop at this location.

  1. start the debugging session when this gem is required.

require 'debug' start debgging but don't stop the program (== RUBY_DEBUG_NONSTOP mode).

and the difference between then is the timing of debugging session starts:

  • the 1st options starts at the first binding.bp call
  • the 2nd one starts whenever this lib is required (which is the boot time in Rails app).

I think the 1st option makes more sense and I'd do it like I mentioned in #65 (comment)

@ko1
Copy link
Collaborator

ko1 commented Jun 7, 2021

I think the 1st option makes more sense

Could you explain why?

@st0012
Copy link
Member Author

st0012 commented Jun 7, 2021

activating the debug session with require "debug" means that Rails users (or anyone uses Bundler.require) have gem "debug" in their Gemfiles could activate the session without knowing it.

I know asking them to add , require: false can workaround this issue, but

gem "debug", require: false

# wherever I want to debug
require "debug" # start the session
binding.bp # and add a breakpoint

still has more code than

gem "debug" # require files & define binding.bp, but don't start anything

# wherever I want to debug
binding.bp # start the session and add a breakpoint

@ko1
Copy link
Collaborator

ko1 commented Jun 7, 2021

You are right. How about disadvantages?

@st0012
Copy link
Member Author

st0012 commented Jun 8, 2021

I don't see a clear downside in this approach. shouldn't

gem "debug"

binding.bp

essentially the same as this?

require "debug"

DEBUGGER__.console

# and later
binding.bp

the only difference seems to be that we'll patch Binding right after require "debug" instead of DEBUGGER__.console.

can you point out anything I miss here?

@ko1
Copy link
Collaborator

ko1 commented Jun 8, 2021

#65 (comment)

The drawback is, explicit debug start will track the program execution:

Thread creation
Code loading including eval (store loaded Ruby code to show)
And more (not yet)

@st0012
Copy link
Member Author

st0012 commented Jun 8, 2021

I don't understand. since these start in Session#initialize, doesn't DEBUGGER__.console starts them as well?

@ko1
Copy link
Collaborator

ko1 commented Jun 8, 2021

In #65 (comment) I showed 3 points.

  1. Load debug functionality: Loading the debug classes/modules/methods
  2. Start debug: load debug functionality and start tracking the information for debugging. Also load .rdbgrc for configuration.
  3. Stop in a debugger context:
  • requiring debig start (1)
  • requiring debug/run (or exe/rdbg, console and so on) start (1) and (2)
  • binding.bp without starting (2), start (2) and (3).

So after binding.bp, information is tracked. However, the code running before first binding.bp without (2) information is not collected. And as I wrote,

In many cases it is not a problem.

In other words, there is a problem in a few cases.

@st0012
Copy link
Member Author

st0012 commented Jun 10, 2021

because DEBUGGER__.console also stops the program, so the current flow looks like this, right?

require "debug" # code loaded (1)

# nothing tracked here

DEBUGGER__.console # start tracking (2) + stop the program (3)

# things are tracked

binding.bp # stop the program (3)

to users, it means that "before the program is first stopped, nothing is being tracked".

so if we can make binding.bp do both (2) and (3) properly, it's still the same behavior users

require "debug" # code loaded (1)

# nothing tracked here

binding.bp # start tracking (2) + stop the program (3)

# things are tracked

binding.bp # stop the program (3)

I don't see why this will cause new problems?

or are you worrying about the tracking won't be explicit to users after the change?

if that's the case, I think the problem would be the APIs being confusing when users use it like byebug and pry:

(speaking as someone who doesn't know this project very well)

  • I don't clearly understand what DEBUGGER__.console tracks. I just know it stops the program, so it seems to be the same as binding.bp.
  • I rarely need the information before binding.bp, so I don't want to call DEBUGGER__.console before using it.

Potential Solution

I think we can reduce the confusion by making the API more flexible:

  • allow using binding.bp without calling DEBUGGER__.console.
  • when this happens, it performs both (2) & (3). it also warns users that to get the information prior the breakpoint, they also need to start the tracking manually.
  • add something like DEBUGGER__.track that starts tracking but doesn't stop the program.

so eventually the flow would look like:

Without Prior Tracking

for using like byebug or binding.pry

require "debug" # code loaded (1)

# nothing tracked here

binding.bp # start tracking (2) + stop the program (3)
# also reminds users about tracking

# things are tracked

binding.bp # stop the program (3)

With Prior Tracking

require "debug" # code loaded (1)

# nothing tracked here

DEBUGGER__.track # start tracking (2)

# things are tracked

binding.bp # stop the program (3)

@ko1
Copy link
Collaborator

ko1 commented Jun 15, 2021

or just .start I'm thinking.

because DEBUGGER__.console also stops the program, so the current flow looks like this, right?

This is why there is -n option on rdbg.

@st0012
Copy link
Member Author

st0012 commented Jun 15, 2021

This is why there is -n option on rdbg.

but it's not accessible from application commands like rails or rspec. I think in the future most of the web developer users would not start the debugging session with the rdbg, so -n won't be an option to them.

@ko1
Copy link
Collaborator

ko1 commented Jun 16, 2021

not important but they are also -n:

  • nonstop option for console and so on.
  • RUBY_DEBUG_NONSTOP envval.

I'm thinking to start debug session (tracking) only with require 'debug'.

@ko1
Copy link
Collaborator

ko1 commented Jun 16, 2021

    1. start tracking
    • require 'debug'
    • gem 'debug' in Gemfile with Bundler.require
    1. start console
    • a. require 'debug/run' or require 'debug/open'
    • b. console for local console
    • c. open methods for remote
    • d. binding.bp

@st0012
Copy link
Member Author

st0012 commented Jun 17, 2021

    1. start tracking
    • require 'debug'
    • gem 'debug' in Gemfile with Bundler.require
    1. start console
    • a. require 'debug/run' or require 'debug/open'
    • b. console for local console
    • c. open methods for remote
    • d. binding.bp

yeah I think this will make starting the debugger a lot easier 👍
since this may conflict with the DAP work, I think it's better to be implemented by you as well?

@st0012
Copy link
Member Author

st0012 commented Jun 20, 2021

@ko1 I'm going to give it a shot 🙂

@st0012
Copy link
Member Author

st0012 commented Jun 20, 2021

@ko1 can you help me verify if these scenarios are correct?

Scenarios

-r debug/run

Script:

a = 1 # stops here
b = 2
c = 3
d = 4

Command: $ ruby -Ilib -r debug/run target.rb

-r debug/open

Script:

a = 1 # stops here
b = 2
c = 3
d = 4

Commands:

  • $ ruby -Ilib -r debug/open target.rb
  • $ exe/rdbg --attach

require 'debug/run'

Script:

a = 1
b = 2
require "debug/run" # stops here
c = 3
d = 4

Command: $ ruby -Ilib target.rb

require 'debug/open'

Script:

a = 1
b = 2
require "debug/open" # stops here
c = 3
d = 4

Commands:

  • $ ruby -Ilib target.rb
  • $ exe/rdbg --attach

DEBUGGER__.console

Script:

require "debug"
a = 1
b = 2
DEBUGGER__.console # stops here
c = 3
d = 4

Command: $ ruby -Ilib target.rb

DEBUGGER__.open

Script:

require "debug"
a = 1
b = 2
DEBUGGER__.open # stops here
c = 3
d = 4

Commands:

  • $ ruby -Ilib target.rb
  • $ exe/rdbg --attach

binding.bp

Script:

require "debug"
a = 1
b = 2
binding.bp # stops here
c = 3
d = 4

Command: $ ruby -Ilib target.rb

@ko1
Copy link
Collaborator

ko1 commented Jun 28, 2021

.console and .open should be at the beginning of the program, I assumed.

@st0012
Copy link
Member Author

st0012 commented Jun 28, 2021

does that mean in the future those 2 methods won't stop the program anymore? (which I'd prefer)

@ko1
Copy link
Collaborator

ko1 commented Jul 2, 2021

I also find that entering debug console with C-c should be allowed with require 'debug'. Now I'm making it to allow entering the console.

@ko1
Copy link
Collaborator

ko1 commented Jul 6, 2021

I found that the application should terminate with SIGINT if user doesn't enable debugger explicitly.

  • Implicit
    • gem debug in Gemfile and require require 'debug' by Bundler.require
    • require 'debug'
  • Explicit
    • use rdbg command
    • require 'debug/run'
    • require 'debug/open'

Allowed operations:

  • Implicit
    • stop on binding.bp like binding.pry/irb
  • Explicit
    • stop on C-c
    • stop at the beggining
  • Not sure
    • (1) start tracing (sources / threads / ...)
    • (2) load ~/.rdbginit

@ko1
Copy link
Collaborator

ko1 commented Jul 6, 2021

Now I'm thinking to enable (1) and (2) even if it is implicit enabling.

@ko1
Copy link
Collaborator

ko1 commented Jul 6, 2021

To control them, I'm thinking DEBUGGER__::start() method:

# debug/session.rb

# start debugger functionality.
#
#  trace_loading: start tracing loading files.
#  trace_threading: start tracing thread creation/termination
#  load_init: allow to load init
#  suspend_on_sigint: enter debug session with C-c
#  nonstop: Do not stop at the beggining of main script
def self.start trace_loading: true, 
               trace_threading: true,
               load_init: true,
               suspend_on_sigint: false,
               nonstop: true
end

def self.run
  start suspend_on_sigint: true, nonstop: true
end
# debug.rb
require 'debug/session'
DEBUGGER__::start

# debug/run.rb
require 'debug/session'
DEBUGGER__::run

# debug/open.rb
require 'debug/session'
DEBUGGER__::open

@ko1
Copy link
Collaborator

ko1 commented Jul 7, 2021

I'm not sure anybody wants to use trace_loading and trace_threading, so now I don't introduce it.

@ko1 ko1 closed this as completed in 887a4e2 Jul 7, 2021
@st0012
Copy link
Member Author

st0012 commented Jul 7, 2021

@ko1 thank you!

@ko1
Copy link
Collaborator

ko1 commented Jul 7, 2021

Please try it and tell me how it's good or not.

@st0012
Copy link
Member Author

st0012 commented Jul 7, 2021

@ko1 just tested in my Rails project and it worked great 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants