-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
v8_inspector: Support the V8 debugger from within VM contexts #7593
Comments
Repro:
|
@cpojer are you running Node with the |
@ide yes, please see jestjs/jest#1652 for more details. |
Quoting @bmeck (https://twitter.com/bradleymeck/status/790571493579116546):
|
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. |
The branch from @bmeck looks like a good starting point. This would be a really neat contribution. /cc @nodejs/v8-inspector. |
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. |
Adds all Contexts created by the vm module to the inspector. This does permanent tracking of Contexts and syncing the Contexts whenever a new inspector is created. Fixes: nodejs#7593
Are there any news on this issue? |
@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. |
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? |
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. |
@eugeneo see comment in that PR |
Any news on this? I got here from jestjs/jest#1652 (comment) |
This enables inspector support for contexts created using the vm module. Fixes: nodejs#7593 Fixes: nodejs#12096 Refs: nodejs#9272
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 The documentation would need to be very clear about the potential for memory leaks, but I think the "experimental" designation on the Any thoughts on this approach? |
ping. This issue is still causing JEST tests to not trigger on |
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 |
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...] |
Based on work by Bradley Farias <bradley.meck@gmail.com> and Eugene Ostroukhov <eostroukhov@chromium.org>. TODO: V8's console is injected unconditionally, which may be not desirable. Fixes: nodejs#7593 Refs: nodejs#9272
Downgrade to 7.10.1 to work-around the issue. Keep watching on this. |
@cpojer Maybe FB should donate a developer to the node project for a few months so we can get somewhere :D |
Closing this since Node 8.4.0 supports attaching to vm contexts out of the box. |
Source maps are ignored in chrome dev tools for code that is evaluated using |
I was trying to get jest debugging working and ran into issues. Switched to Node 9.7.1 and the issue went away, breakpoints were hit. Related: nodejs/node#7593
* Update Troubleshooting.md I was trying to get jest debugging working and ran into issues. Switched to Node 9.7.1 and the issue went away, breakpoints were hit. Related: nodejs/node#7593 * Update Troubleshooting.md
Copy executionContext into global variable. This is ugly, because it means that identically-named properties of different execution contexts can step on each other in subsequent calls to model.prepare. This is ok-ish, because SCION support for node --inspect is a development-only feature. This will not affect production uses of SCION. One day I may patch require to accept an executionContext argument, and use vm.runInContext inside of Module._compile.
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: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).The text was updated successfully, but these errors were encountered: