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: fix permission inspector crash #53389

Merged

Conversation

theanarkh
Copy link
Contributor

Fixes: #53385

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

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. labels Jun 8, 2024
@theanarkh theanarkh force-pushed the fix_permission_inspector_crash branch from 9e5e24a to 7573b6b Compare June 8, 2024 18:36
@cola119
Copy link
Member

cola119 commented Jun 9, 2024

cc @eugeneo

@cola119 cola119 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2024
@nodejs-github-bot
Copy link
Collaborator

test/parallel/test-permission-inspector-brk.js Outdated Show resolved Hide resolved
test/parallel/test-permission-inspector-brk.js Outdated Show resolved Hide resolved
@eugeneo
Copy link
Contributor

eugeneo commented Jun 9, 2024 via email

@theanarkh
Copy link
Contributor Author

I will add permission check in node_contextify.cc to fix this(Node.js will run into EvaluateMachine of node_contextify.cc when setting --eval flag).

@theanarkh theanarkh force-pushed the fix_permission_inspector_crash branch 4 times, most recently from 4a8cf55 to ac0f283 Compare June 9, 2024 17:59
@theanarkh theanarkh force-pushed the fix_permission_inspector_crash branch from ac0f283 to 5fee021 Compare June 9, 2024 18:00
@cola119 cola119 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 10, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 10, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@H4ad H4ad added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2024
@nodejs-github-bot nodejs-github-bot merged commit 430c026 into nodejs:main Jun 10, 2024
57 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 430c026

targos pushed a commit that referenced this pull request Jun 20, 2024
PR-URL: #53389
Fixes: #53385
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
PR-URL: nodejs#53389
Fixes: nodejs#53385
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
PR-URL: nodejs#53389
Fixes: nodejs#53385
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
@RafaelGSS RafaelGSS added the permission Issues and PRs related to the Permission Model label Jun 26, 2024
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
PR-URL: #53389
Fixes: #53385
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
PR-URL: #53389
Fixes: #53385
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol needs-ci PRs that need a full CI run. permission Issues and PRs related to the Permission Model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segmentation fault when using inspector on experimental permission enabled process
8 participants