★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/32672Nouvelle recherche | Portail wanayoo
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

src: initialize inspector before RunBootstrapping() #32672

Closed

Conversation

Copy link
Member

@addaleax addaleax commented Apr 5, 2020

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)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ inspector labels Apr 5, 2020
@addaleax addaleax mentioned this pull request Apr 5, 2020
4 tasks
src/inspector_io.cc Outdated Show resolved Hide resolved
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)
@addaleax addaleax force-pushed the inspector-init-createenvironment branch from 0979bd5 to 4ce0133 Compare Apr 5, 2020
src/node.h Show resolved Hide resolved
@addaleax addaleax added the wip label Apr 5, 2020
@addaleax addaleax removed the wip label Apr 5, 2020
@addaleax
Copy link
Member Author

@addaleax addaleax commented Apr 5, 2020

@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?

Copy link
Member

@joyeecheung joyeecheung left a comment

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

@addaleax addaleax added the author ready label Apr 6, 2020
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 6, 2020

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Apr 6, 2020

@addaleax
Copy link
Member Author

@addaleax addaleax commented Apr 7, 2020

Landed in 8aa7ef7

addaleax added a commit that referenced this issue Apr 7, 2020
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>
@addaleax addaleax closed this Apr 7, 2020
@addaleax addaleax deleted the inspector-init-createenvironment branch Apr 7, 2020
BethGriggs pushed a commit that referenced this issue Apr 14, 2020
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>
@richardlau
Copy link
Member

@richardlau richardlau commented Apr 20, 2020

It looks like this PR is making cctest crash when coverage is enabled (see #32912).

@targos targos added the backport-requested-v12.x label Apr 25, 2020
@addaleax addaleax added backport-open-v12.x and removed backport-requested-v12.x labels Sep 17, 2020
addaleax added a commit to addaleax/node that referenced this issue Sep 23, 2020
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>
addaleax added a commit that referenced this issue Sep 23, 2020
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>
@addaleax addaleax added backported-to-v12.x and removed backport-open-v12.x labels Sep 23, 2020
@codebytere codebytere mentioned this pull request Sep 28, 2020
codebytere added a commit to electron/electron that referenced this issue May 20, 2021
codebytere added a commit to electron/electron that referenced this issue May 31, 2021
codebytere added a commit to electron/electron that referenced this issue May 31, 2021
codebytere added a commit to electron/electron that referenced this issue Jun 8, 2021
codebytere added a commit to electron/electron that referenced this issue Jun 9, 2021
codebytere added a commit to electron/electron that referenced this issue Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready backported-to-v12.x c++ inspector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants