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

test: workaround for V8 8.1 inspector pause issue #66

Merged
merged 1 commit into from
Mar 16, 2020

Conversation

sthagen
Copy link
Owner

@sthagen sthagen commented Mar 16, 2020

test-inspector-multisession-ws and test-inspector-break-when-eval
will be affected by an upstream bug when we upgrade V8 to 8.1. The bug
is caused when the Inspector sets a pause at the start of a function
compiled with CompileFunctionInContext, but that function hasn't been
executed yet.

On both tests, this issue is triggered by pausing while in C++ executing
LookupAndCompile, which is called by requiring internal modules while
running console.log. To eliminate this issue in both tests, we add an
extra console.log to ensure we only pause we required all internal
modules we need. On test-inspector-break-when-eval, we also need to
start the child process with --inspect-brk instead of --inspect to
ensure the test is predictable (this test would occasianlly fail on
slower machines, when console.log doesn't run fast enough to finish
after emitting Runtime.consoleAPICalled and before the parent process
sending Runtime.evaluate message.

Ref: https://bugs.chromium.org/p/v8/issues/detail?id=10287

PR-URL: nodejs#32234
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=10287
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Jiawen Geng technicalcute@gmail.com
Reviewed-By: Michaël Zasso targos@protonmail.com

Checklist
  • 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

`test-inspector-multisession-ws` and `test-inspector-break-when-eval`
will be affected by an upstream bug when we upgrade V8 to 8.1. The bug
is caused when the Inspector sets a pause at the start of a function
compiled with `CompileFunctionInContext`, but that function hasn't been
executed yet.

On both tests, this issue is triggered by pausing while in C++ executing
LookupAndCompile, which is called by requiring internal modules while
running `console.log`. To eliminate this issue in both tests, we add an
extra `console.log` to ensure we only pause we required all internal
modules we need. On `test-inspector-break-when-eval`, we also need to
start the child process with `--inspect-brk` instead of `--inspect` to
ensure the test is predictable (this test would occasianlly fail on
slower machines, when console.log doesn't run fast enough to finish
after emitting `Runtime.consoleAPICalled` and before the parent process
sending `Runtime.evaluate` message.

Ref: https://bugs.chromium.org/p/v8/issues/detail?id=10287

PR-URL: #32234
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=10287
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@sthagen sthagen merged commit 7ab3cf0 into sthagen:master Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants