[inspector] Assertion errorΒ #14745
Copy link
Copy link
Closed
Labels
inspectorIssues and PRs related to the V8 inspector protocol.Issues and PRs related to the V8 inspector protocol.
Description
- Version: v8.2.1
- Platform: armv7l GNU/Linux
- Subsystem: inspector
Executing the following code and attaching the debugger causes an assertion Error:
const inspector = require('inspector');
inspector.open(9229, '0.0.0.0');
console.log(inspector.url());
const session = new inspector.Session();
session.connect();
session.on('inspectorNotification', (message) => console.log(message.method));
console.log('ready for inspecting');
setInterval(()=>{},1000); //edit: added later
output:
Debugger listening on ws://0.0.0.0:9229/ac478aae-7939-4f2e-ad21-c837da5abbfc
For help see https://nodejs.org/en/docs/inspector
ws://0.0.0.0:9229/ac478aae-7939-4f2e-ad21-c837da5abbfc
ready for inspecting
Debugger attached.
node[20898]: ../src/inspector_agent.cc:478:void node::inspector::NodeInspectorClient::connectFrontend(node::inspector::InspectorSessionDelegate*): Assertion `(channel_) == (nullptr)' failed.
I'm trying to open an inspector session, and receive a callback once the debugger has been attached and flow control has been passed over.
Metadata
Metadata
Assignees
Labels
inspectorIssues and PRs related to the V8 inspector protocol.Issues and PRs related to the V8 inspector protocol.
Activity
addaleax commentedon Aug 10, 2017
@nodejs/v8-inspector
Fwiw I canβt reproduce this on x64 Linux.
jeroenvollenbrock commentedon Aug 10, 2017
I've just tested this on OSX 10.12.5 (node 8.2.1) as well. (I realized i forgot to add a
setInterval(()=>{},10000);to the initially posted source to keep node running)jeroenvollenbrock commentedon Aug 10, 2017
As a side-note, what i'm actually trying to achieve is to create a promisified version of
inspector.open(9229, '0.0.0.0', true);which resolves after flow control has been passed to the debugger instead of blocking the main loop until this happens, so we can incorporate it into the App SDK of our product. This would allow us to first setup all app api's and security, drop privileges, then start the inspector and communicate the session ID/url over the IPC socket to the core process, and finally wait for a debugger to be attached before continuing to run the user-specified code.eugeneo commentedon Aug 10, 2017
Can you provide more details?
This assertion fails when there is already a debugger attached - but there should be a JS exception raised instead.
jeroenvollenbrock commentedon Aug 11, 2017
I'm using no command line flags and attaching the chrome devtools. Right after attaching, the crash occurs.
TimothyGu commentedon Aug 11, 2017
The bug is specific to the following sequence:
I've got a WIP in https://github.com/TimothyGu/node/tree/inspector-work to fix this issue, but eventually gave up because of the possibility of race conditions.
We can only know whether WS inspector attachment succeeded when it does
Agent::Connect()(after the modifications in b01193f344b579248034df7fb580ab17b65ce930 to that method), but the WS server works in such a way thatAgent::Connect()is not done synchronously in the WS message handler itself. Rather, the connection task is added to a queue and done asynchronously. However, we have to return the results of connection (whether it succeeded) synchronously from the message handler (InspectorIo::DispatchMessages()which callsInspectorIoDelegate::StartSession()).Now, we can check in the message handler whether it's possible to attach (which is what I did) and responds to the WS message accordingly. But there can be a delay between the addition to the task queue in the handler and the actual connection and check whether
Agenthas been connected or not, since the WS server runs on a different thread. E.g. after the addition of a sleep() (designed to artificially lengthen the time it can take before the task is processed), it's possible for the following to happen:My work was based on the premise that we want to allow having IO thread open but unattached and allowing JS binding connection. It'd be simpler if we just forbid any JS binding connection while the IO thread is open, regardless of whether a debugger has been attached or not. Although that's not ideal, it seems to be the only way out of this without major refactors to the WS server.
jeroenvollenbrock commentedon Aug 11, 2017
@TimothyGu Thank you for the clarification! I think i misunderstood the documentation while trying to build something that would meet my needs, is something like that possible at all right now?
eugeneo commentedon Aug 11, 2017
@TimothyGu
This was an explicit requirement when JS bindings were added. Chrome will soon support multiple DevTools protocol clients and the same support will be ported to Node once Node moves to compatible V8 version (I don't know version number yet).
Also, I am working on an experimental branch that makes the WS use JS bindings session instead of special code in the native part. It should make issues like this more manageable. I am intending to create a PR within the next couple weeks.
TimothyGu commentedon Aug 11, 2017
Yes, that should help things. Looking forward to it!
TimothyGu commentedon Aug 12, 2017
@jeroenvollenbrock I don't see anything impossible with what you are trying to do, though I'm unsure why you are trying to use the JS binding
SessionAPI withinspector.open()?inspector.openis already asynchronous, but we don't currently have a way to pass events from the Inspector WS server to JS so you can't get notification when the server starts.jeroenvollenbrock commentedon Aug 14, 2017
@TimothyGu I would like to use inspector.open with
wait=true, but i need to pass the url/id to another process as well (stdout/err is not accessible at this time). This is currently impossible, because the ID/url is only accessible after invoking.open, but as the.openblocks the eventloop until a debugger is connected, it's not possible to pass the id up before the debugger connects. Setting thewaitparameter to false would make this possible, but also makes it impossible to wait for the debugger to be connected before resuming execution of the code that needs to be debugged. Therefore i was hoping i could make a promisified version of the open method, that does not block the entire eventloop until the debugger connects, but just resolves a promise. The docs gave me the impression this should be possible by starting/connecting a Session as well, and resolving the promise after the firstinspectorNotificationevent got delivered.TimothyGu commentedon Aug 14, 2017
@jeroenvollenbrock Ah I see. Sorry,
Sessionis unrelated toopen(), and ifopen()does not work for your purpose I don't think we have a way to make that possible for now :(eugeneo commentedon Aug 14, 2017
This is a bug. What happens is that the WS code path does not fully account for the possibility of the JS bindings session.
eugeneo commentedon Jan 31, 2018
Verified on Node v8.9.2 - session.connect() now throws a JS exception as expected. There will be no exception once concurrent sessions are supported.