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

--inspect race condition #25215

Closed
canonic-epicure opened this issue Dec 25, 2018 · 10 comments
Closed

--inspect race condition #25215

canonic-epicure opened this issue Dec 25, 2018 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. inspector Issues and PRs related to the V8 inspector protocol stale

Comments

@canonic-epicure
Copy link

The activation of the --inspect cmd option is subject to race condition. To reproduce:

  1. Create a file, called debugger.js with the content:
debugger

setTimeout(() => { debugger }, 10)

setTimeout(() => { debugger }, 50)

setTimeout(() => { debugger }, 100)

setTimeout(() => { debugger }, 200)

setTimeout(() => { debugger }, 300)
  1. Launch it first with:
    node --inspect-brk debugger.js
    to open the Inspector window in the Chrome dev tools

  2. Then launch it several times with:
    node --inspect debugger.js

  3. Observe most of the times ALL breakpoints are ignored, sometimes ALL are activated, sometimes only the one with 300ms delay.

  4. Expected behavior - all breakpoints are expected to always be honored, including the very 1st one.

@addaleax addaleax added the inspector Issues and PRs related to the V8 inspector protocol label Dec 26, 2018
@juanarbol
Copy link
Member

I tried to recreate this, I found that process is ending before any timer is called. Using --inspect flag, process close without touching any breakpoint; instead of using --inspect-brk is going to "pause" my node process (as expected) and stop on every breakpoint.

PD: I just put this at the end of the file process.on('exit', () => console.info(process.uptime()))

@canonic-epicure
Copy link
Author

Any news about this issue? I have to add an ugly "wait for 300ms" rule in our testing tool Siesta to workaround it.

@canonic-epicure
Copy link
Author

Should be very easy to fix, as the --inspect-brk option already has the needed behavior - it waits for the debugger instance to be attached - this is what is missing in the --inspect

@eugeneo
Copy link
Contributor

eugeneo commented Feb 1, 2019

I am not sure what is the bug here. debugger is supposed to be noop when no debug sessions are connected. Use --inspect-brk to ensure that frontend has a chance to attach before the debugger statement is reached.

@canonic-epicure
Copy link
Author

debugger is supposed to be noop when no debug sessions are connected.

I do not agree with this. From usability perspective, when I place debugger somewhere and I've indicated that I need a debugging session with --inspect option, I expect the code to stop in that place.

And if the behavior is - "yes, it will stop, but only after 300ms after the process start or so, may be after 100ms if you are lucky" - then I would say its a bug and race condition.

The correct behavior would be:

  • both for --inspect and --inspect-brk - stop the code on the 1st statement and wait for debugger session to attach
  • for --inspect-brk - nothing else, continue waiting for user action
  • for --inspect - run the code (F8)

@jasnell jasnell added the feature request Issues that request new features to be added to Node.js. label Jun 26, 2020
@canonic-epicure
Copy link
Author

canonic-epicure commented Dec 2, 2020

Or rather, the correct behavior should be:

  • when --inspect option is given - encountering the debugger statement in the code should "stop the world" and wait for debugger session to attach
  • same when an unhandled exception (or rejection) is thrown

Any updates on this issue? The experience of debugging simple node scripts is still not optimal.

@benjamingr
Copy link
Member

The thing is @canonic-epicure we get this behavior from Chrome and the Chrome DevTools that need an inspector attached but won't pause anything without an actual debugger connected.

This makes sense and is how debuggers behave in other languages and environments too - you can tell the debugger "always pause when told" and launch with --inspect-brk which pauses until it connects or just tell it "I may connect in the future" with --inspect which doesn't.

@eugeneo
Copy link
Contributor

eugeneo commented Mar 7, 2022

@canonic-epicure the way your frontend should work is:

  1. Start the process with --inspect-brk
  2. Connect and setup the debug session.
  3. Resume the process.
    There is no need to communicate to the user that the target had been suspended at all.

debugger keyword requires V8 to run in a debug mode. I am several years behind the current V8 implementation, but at least it used to be that "debug mode" would disable some optimization and retain some extra info. We could not afford it in all cases when --inspect is passed. Inspector protocol can be used for tracing and profiling that do not always require switching V8 into the debug mode.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Sep 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2022

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as completed Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. inspector Issues and PRs related to the V8 inspector protocol stale
Projects
None yet
Development

No branches or pull requests

6 participants