Skip to content

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 [email protected]
Reviewed-By: Jiawen Geng [email protected]
Reviewed-By: Michaël Zasso [email protected]

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 <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@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