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

inspector-port=0 edge case crash #13499

Closed
refack opened this issue Jun 6, 2017 · 5 comments · Fixed by #13504
Closed

inspector-port=0 edge case crash #13499

refack opened this issue Jun 6, 2017 · 5 comments · Fixed by #13504
Labels
cli Issues and PRs related to the Node.js command line interface. confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol

Comments

@refack
Copy link
Contributor

refack commented Jun 6, 2017

  • Version: 8.1.0
  • Platform: *
  • Subsystem: cli

after 399cb25 there's an edge case that crashes node

D:\code\node$ Release\node.exe --inspect-port=0
> process.debugPort

D:\code\node$ echo %ERRORLEVEL%
-1073741819

D:\code\node$
@refack refack added cli Issues and PRs related to the Node.js command line interface. confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol labels Jun 6, 2017
@sam-github
Copy link
Contributor

This happens on master as well as 8.x and 8.x-staging

@eugeneo
Copy link
Contributor

eugeneo commented Jun 6, 2017

I do not see this on Linux and Mac. Other then Windows OS, is there anything else that might be triggering it?

@sam-github
Copy link
Contributor

I only tried it on my laptop (Ubuntu), and it repros for me, and makes sense.

IsStarted() is confusingly named, it is true if the inspector is initialized, even when it doesn't listen. There is only an ->io() after the inspector starts the listening/IO thread, and that doesn't happen with --inspect-port

@sam-github
Copy link
Contributor

And when I back out the fix in #13504, the test in that PR fails.

@eugeneo
Copy link
Contributor

eugeneo commented Jun 6, 2017 via email

sam-github added a commit to sam-github/node that referenced this issue Jun 9, 2017
Inspector start means that it exists, but doesn't mean it is listening
on a port, that only happens if it is doing I/O (i.e. has an io object).

PR-URL: nodejs#13504
Fixes: nodejs#13499
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this issue Jun 10, 2017
Inspector start means that it exists, but doesn't mean it is listening
on a port, that only happens if it is doing I/O (i.e. has an io object).

PR-URL: #13504
Fixes: #13499
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues and PRs related to the Node.js command line interface. confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants