-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
src: initialize inspector before RunBootstrapping() #32672
Conversation
This is necessary for `--inspect-brk-node` to work, and for the inspector to be aware of scripts created before bootstrapping. Fixes: nodejs#32648 Refs: nodejs#30467 (comment)
0979bd5
to
4ce0133
Compare
@devsnek Sorry, had to make a few changes to get tests to pass (some of which were rather obvious in hindsight) … could you take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I am still skeptical about whether it's necessary to burden users with maintaining InspectorParentHandle across threads through the Environment API, but it is what it is
CI: https://ci.nodejs.org/job/node-test-pull-request/30502/ (:heavy_check_mark:) |
Landed in 8aa7ef7 |
This is necessary for `--inspect-brk-node` to work, and for the inspector to be aware of scripts created before bootstrapping. Fixes: #32648 Refs: #30467 (comment) PR-URL: #32672 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This is necessary for `--inspect-brk-node` to work, and for the inspector to be aware of scripts created before bootstrapping. Fixes: #32648 Refs: #30467 (comment) PR-URL: #32672 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
It looks like this PR is making |
This is necessary for `--inspect-brk-node` to work, and for the inspector to be aware of scripts created before bootstrapping. Fixes: nodejs#32648 Refs: nodejs#30467 (comment) PR-URL: nodejs#32672 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This is necessary for `--inspect-brk-node` to work, and for the inspector to be aware of scripts created before bootstrapping. Fixes: #32648 Refs: #30467 (comment) Backport-PR-URL: #35241 PR-URL: #32672 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This is necessary for
--inspect-brk-node
to work, and for theinspector to be aware of scripts created before bootstrapping.
Fixes: #32648
Refs: #30467 (comment)
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes