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

bootstrap: clean up inspector console methods during serialization #44279

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

joyeecheung
Copy link
Member

Some console methods are created by the V8 inspector after
an inspector client is created, reset them to undefined during
sereialization to avoid holding on to invalid references in
the snapshot. V8 will take care of the re-initialization when
another inspector client is created during deserialization.

Fixes: nodejs/node-v8#237

Some console methods are created by the V8 inspector after
an inspector client is created, reset them to undefined during
sereialization to avoid holding on to invalid references in
the snapshot. V8 will take care of the re-initialization when
another inspector client is created during deserialization.
@nodejs-github-bot nodejs-github-bot added console Issues and PRs related to the console subsystem. needs-ci PRs that need a full CI run. labels Aug 18, 2022
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the review wanted PRs that need reviews. label Aug 19, 2022
@joyeecheung
Copy link
Member Author

cc @nodejs/inspector @nodejs/console @nodejs/startup

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Sep 6, 2022

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. and removed review wanted PRs that need reviews. labels Sep 7, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 7, 2022
@nodejs-github-bot nodejs-github-bot merged commit f0cf100 into nodejs:main Sep 7, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in f0cf100

Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
Some console methods are created by the V8 inspector after
an inspector client is created, reset them to undefined during
sereialization to avoid holding on to invalid references in
the snapshot. V8 will take care of the re-initialization when
another inspector client is created during deserialization.

PR-URL: nodejs#44279
Fixes: nodejs/node-v8#237
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
Some console methods are created by the V8 inspector after
an inspector client is created, reset them to undefined during
sereialization to avoid holding on to invalid references in
the snapshot. V8 will take care of the re-initialization when
another inspector client is created during deserialization.

PR-URL: #44279
Fixes: nodejs/node-v8#237
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Sep 26, 2022
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
Some console methods are created by the V8 inspector after
an inspector client is created, reset them to undefined during
sereialization to avoid holding on to invalid references in
the snapshot. V8 will take care of the re-initialization when
another inspector client is created during deserialization.

PR-URL: #44279
Fixes: nodejs/node-v8#237
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@juanarbol
Copy link
Member

The introduced test test/parallel/test-snapshot-console.js is not working in the v16.x release line; it is complaining about the --experimental-async-stack-tagging-api flag.

@szuend
Copy link
Contributor

szuend commented Oct 18, 2022

The --experimental-async-stack-tagging-api flag has been enabled by default with V8 10.6 and we want to remove the flag with V8 10.9. What would be the process to remove the flag again from the test upstream in node?

@targos
Copy link
Member

targos commented Oct 18, 2022

@suend I think you can open a PR against the main branch, as we now have V8 10.7 in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshot test failures
8 participants