Fix event timing

[TimothyGu]

Fixes the symptoms documented at nodejs/node#17336. This commit contains multiple parts that are hard to separate into separate commits/PRs, but I've documented the bits that I changed below:

• Use process.execPath instead of hardcoding 'node'
• Make sure to call done() in the foreground() callback; doing this used to segfault for unknown reasons (presumably inside V8), but will no longer do so after the timing fixes in this PR
• Listen for Inspector events before enabling Profiler/Runtime/Debugger, since events will start flowing in as soon as those are enabled
• Attempt to enable Profiler/Runtime/Debugger before runIfWaitingForDebugger, but do not expect them to complete before runIfWaitingForDebugger finishes
• Wait for the initial pause brought about by --inspect-brk before doing anything further, like starting coverage tracking
• Ignore executionContextDestroyed signals for non-main contexts (e.g. VM contexts)
• Use Promises more extensively

[bcoe]

was not waiting on the Debugger.paused event the root cause of the issues I was seeing? Interesting that this worked for CommonJS files.

[TimothyGu]

was not waiting on the Debugger.paused event the root cause of the issues I was seeing?

Yes, I believe so.

Interesting that this worked for CommonJS files.

I agree, with the minor amendment that if you rename c.mjs to d.js the same issue would be seen.

[bcoe]

weirdly, this does not seem to work for process.exit or throw in a script, unless you refactor thusly:

    const {Debugger, Runtime, Profiler} = client
    await Promise.all([
      Profiler.enable(),
      Runtime.enable(),
      Debugger.enable(),
      Runtime.runIfWaitingForDebugger(),
    ])
    await Profiler.startPreciseCoverage({callCount: true, detailed: true})
    await initialPause
    await Debugger.resume()

you can exercise this by running ./bin/c8.js test/fixtures/c.mjs (which should output block coverage information when running properly) -- any idea why that order of operations matters? feels a little nit-picky.

[TimothyGu]

The proposed solution crashes with a segfault here. Will investigate further.

[TimothyGu]

Some more discoveries:

1. The reason why nothing in c.mjs, the main file, gets tracked is that Profiler.startPreciseCoverage was executed after the main file has been parsed. As such, you are correct in discovering that Profiler.startPreciseCoverage needs to be executed as early as possible.
2. However, the V8 used in Node.js 9.x and master crashes with a segmentation fault if Profiler.startPreciseCoverage is received before Debugger.paused -- but not when Profiler.startPreciseCoverage is received, rather it continues normally until when Debugger.paused is expected to be emitted.
3. The V8 used in canary builds (6.4.358-node.0 tested) does not exhibit the crash above, and works fully as expected, due to v8/v8@1420e44.

What I'm going to do is put this PR on hold, before the issue with V8 gets resolved. Then I will look into backporting the V8 commit to Node.js.

[bcoe]

@TimothyGu awesome; worth noting, we have a lot of test coverage related tweaks landing in v8 soon -- I've been doing most of my testing with the canary build of v8/Node.js.

[bcoe]

@TimothyGu when you have two secs, mind updating this with the timing tweaks and I'll land it?

[TimothyGu]

@bcoe Done. Note, that this will need a node binary with nodejs/node#17344 applied for it to work though.

[bcoe]

@TimothyGu thank you; I will test when I get home tonight.

[TimothyGu]

I've just rebased this.

[bcoe]

@TimothyGu awesome; @chrisdickinson and I actually just tested your branch, it's working great.