v8_inspector: Support the V8 debugger from within VM contexts

[ide]

This is a feature request to support Node 6.3.0's V8 inspector capability (node --inspect) from within the vm module's contexts. For example:

vm.runInNewContext(code, sandbox, {
  inspect: 9999, // same as --inspect=9999
});

If it's not possible to make individual contexts inspectable, it'd also meet our needs to make other contexts inspectable if the parent Node process is running with --inspect. Currently code that runs in other contexts isn't debuggable (ex: debugger statements are ignored).

[cpojer]

node --inspect also doesn't stop for debugger; statements.

Repro:

var vm = require('vm');
new vm.Script('debugger;').runInContext(vm.createContext());
debugger;

[ide]

@cpojer are you running Node with the --debug-brk option? You need that so that Node waits for you to connect the Blink Inspector since debugger statements are no-ops unless there is a debugger actively attached.

[cpojer]

@ide yes, please see jestjs/jest#1652 for more details.

[jkrems]

Quoting @bmeck (https://twitter.com/bradleymeck/status/790571493579116546):

basically you need to setup

node/deps/v8_inspector/third_party/v8_inspector/platform/v8_inspector/V8InspectorImpl.cpp

Line 199 in db1087c

void V8InspectorImpl::contextCreated(const V8ContextInfo& info)

properly to work w/ vm module

[bmeck]

can be based upon https://github.com/bmeck/node/tree/inspect-contexts , don't really have time to figure out segfaults currently but would need tests for sure to ensure GC properly works on this.

[ofrobots]

The branch from @bmeck looks like a good starting point. This would be a really neat contribution.

/cc @nodejs/v8-inspector.

[bmeck]

i'll take this over, it seems like there is also a bug with the inspector not being synced the contexts on connect which makes this less trivial since we need to rejig the handshaking.

[oldrich-s]

Are there any news on this issue?

[jkrems]

@czb There is an open PR (#9272), linked above. You can subscribe to it to get notified on updates. Doesn't look like there has been progress in the last few months.

[eugeneo]

@czb this is blocked on deprecating the legacy debugger. Only one "debugger" can connect to V8 - so either the inspector or the legacy debugger. Current node versions support starting up legacy debugger at any time by sending the signal - which means we cannot provide inspector API that are always available.

[eugeneo]

I spent some time trying to add context reporting to Inspector and feel like I hit the wall (for now).

The core problem is that the Node contexts do not behave in the same way as V8 Inspector (that is coming from the browser) was designed. Browser contexts lifetime is tied to likes of frames and workers so the Inspector can be notified when the context is going away (e.g. while the context is still fully alive). Node context "dies" when it is collected by GC so the Inspector in its current form cannot work with the context at that point.

What it boils down to is that this assert is triggered on GC - https://github.com/nodejs/node/blob/master/deps/v8/src/inspector/inspected-context.cc#L28 :)

There is a chance that this issue will need to be fixed upstream before the Node side can be implemented.

@bmeck do you mind me taking over this issue?

[bmeck]

@eugeneo does that mean I can continue #9272 / can you comment if the problems still apply?

[eugeneo]

The problems still apply... What I am trying to do is similar to what you have there - only difference is that I am relying on inspector agent to always be there so there's no need to have a vector of contexts.

[bmeck]

@eugeneo see comment in that PR

[tnrich]

Any news on this? I got here from jestjs/jest#1652 (comment)
Thank you for the hard work!

[jgoz]

Given that Node and V8-Inspector have incompatible expectations about context lifecycles (Node expects GC cleanup, while Inspector expects explicit cleanup), perhaps we can approach this issue differently.

Rather than automatically attaching every VM context to the inspector, we could expose an API for attaching/detaching manually. I'm thinking something like the following:

var inspector = require('inspector');
var vm = require('vm');

var context = vm.createContext();

inspector.attachContext(context); // this will prevent GC cleanup of 'context'
// do something with 'context'
inspector.detachContext(context); // allow GC cleanup again

The use case I had in mind was library authors who use VM contexts for executing user code (e.g., test runners) because context lifecycles are finite and well-defined: create context, attach context, execute test, detach context.

But this could also be generally useful for debugging sessions when VM contexts are involved. Inserting an inspector.attachContext statement is somewhat analogous to inserting a console.log statement to print a value during a debugging session. The difference is that the former could cause memory leaks if not cleaned up.

The documentation would need to be very clear about the potential for memory leaks, but I think the "experimental" designation on the inspector module is advantageous here. It implies "here be dragons, pay attention".

Any thoughts on this approach?

[michahell]

ping. This issue is still causing JEST tests to not trigger on debugger; statements: jestjs/jest#1652

[aschrijver]

I just referred to this issue from stackoverflow after spending more than a day on setting up debugging, without results! FYI: https://stackoverflow.com/questions/45056952/debugging-jest-unit-tests-with-breakpoints-in-vs-code-with-react-native

[aschrijver]

Are there any debuggers besides Chrome and VS Code that I can use to debug using V8 or should I downgrade and bide my time?

[UPDATE: I decided to downgrade to node 7 and now debugging in VS Code is a breeze! See details...]

[wangxuepeng]

Downgrade to 7.10.1 to work-around the issue. Keep watching on this.

[hakunin]

@cpojer Maybe FB should donate a developer to the node project for a few months so we can get somewhere :D

[jgoz]

@eugeneo This is closed now that #14465 is merged, right?

[ide]

Closing this since Node 8.4.0 supports attaching to vm contexts out of the box.

[jbeard4]

Source maps are ignored in chrome dev tools for code that is evaluated using vm.runInContext.