Skip to content
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

How can a debug adapter know if VS Code has finished sending all breakpoints? #4

Closed
felixfbecker opened this issue Dec 20, 2015 · 10 comments
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@felixfbecker
Copy link

setBreakpointsRequest gets called for every source file that has break points. I have to register them to the debugger engine and then issue a run command to the debugger engine. But if setBreakpointsRequest is called multiple times, how can I ever know if that was it or if there are still more break points in more source files coming? How can I know when to tell the debugger engine to actually run the program?

@felixfbecker
Copy link
Author

So I found this in the Go adapter

protected setExceptionBreakPointsRequest(response: DebugProtocol.SetExceptionBreakpointsResponse, args: DebugProtocol.SetExceptionBreakpointsArguments): void {
        console.log("ExceptionBreakPointsRequest");
        // Wow - this is subtle - it appears that this event will always get 
        // sent during intiail breakpoint initialization even if there are not
        // user breakpoints - so we use this as the indicator to signal 
        // that breakpoints have been set and we can continue
        this.signalInitialBreakpointsSet();
        this.sendResponse(response);
        console.log("ExceptionBreakPointsResponse");
    }

is this the right approach?

@lukehoban
Copy link

FWIW - the implementation in the Go debug adapter is based on https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/nodeDebug.ts#L1117, which triggers the completion of the the debug session initialization during the setExceptionBreakPointsRequest handler.

It's certainly subtle that this would be the (only?) way to be sure you are ready to complete the initialization of the debug session.

@weinand weinand added this to the Jan 2016 milestone Dec 23, 2015
@weinand weinand added the feature-request Request for new features or functionality label Dec 23, 2015
@weinand weinand self-assigned this Dec 23, 2015
@weinand
Copy link
Contributor

weinand commented Dec 23, 2015

@felixfbecker @lukehoban atm the only (correct) way to know that all persisted breakpoints have been sent to the debug adapter is to wait for the next non-setBreakpoints request. This is a bit of a pain but correct. A (non-correct) shortcut is to use the setExceptionBreakPointsRequest.

The reason why there is currently no better way to do this, is that the mono debug adapter and the gdb debug adapter both request the breakpoints before a launchRequest or attachRequest is issued. So in this case "launch" or "attach" were a natural indication that all breakpoints have been set. Only for node it was necessary to delay this because there is no debugger running before a "launch" or "attach".

I propose that we add a new request that indicates the end of the "initialize" phase.

@felixfbecker
Copy link
Author

@weinand I also think there should be a seperate request. So, for my debug adapter that is derived from the node DebugSession class, is there any event that gets fired for all requests, that I can bind to, so I can check if it's the first non-breakpoint request? I dont want to add that check to every single one of my request methods. Should I overwrite dispatchRequest with that check and a call to super?

And just curious, how can a debugadapter run before there is a launch request?!
Also, in the case of PHP/XDebug neither Attach nor Launch really fit. The debug adapter starts, but doesn't launch anything, it just listens on port 9000. Then the user makes a HTTP request to his webserver, XDebug conects to port 9000, and the debug session starts. Ideally, there should be a "listen" request besides "attach" and "launch"...

@weinand
Copy link
Contributor

weinand commented Jan 6, 2016

@felixfbecker, @lukehoban I'm going to add a new request ConfigurationDoneRequestto the protocol. A client (e.g. VSCode) is expected to call this request after all (persisted) configuration requests have been made, that is after SetBreakpointsRequest and SetExceptionBreakpointsRequest.

Any objections or suggestions for a better name?

@felixfbecker
Copy link
Author

@weinand I think it's great 👍

Just to clarify, the sequence is

  • 1 InitializedEvent (at any time)
  • n SetBreakpointRequest
  • 1 SetExceptionBreakpointsRequest
  • potentially other requests that are added in the future
  • 1 ConfigurationDoneRequest

But it would never get sent after a normal SetBreakpointRequest, like when the user changed some settings?

@weinand
Copy link
Contributor

weinand commented Jan 6, 2016

@felixfbecker yes, your sequence is correct and since ConfigurationDoneRequest is a reaction to the InitializedEvent (and not the SetBreakpointsRequest), it will not be sent after a 'normal' SetBreakpointRequest.

@felixfbecker
Copy link
Author

👍

@weinand
Copy link
Contributor

weinand commented Jan 6, 2016

before this can be used, VS Code has to adapt first. I've created microsoft/vscode#1828 for tracking this.

@weinand weinand closed this as completed Jan 6, 2016
weinand added a commit to microsoft/vscode-node-debug that referenced this issue Jan 6, 2016
@weinand
Copy link
Contributor

weinand commented Jan 6, 2016

microsoft/vscode#1828 is fixed. However, since old debug adapters are not implementing the new ConfigurationDoneRequest, they now produce an error on the dev tools console. This error is not visible to the user but is annoying anyway. We should use feature #9 to only send the ConfigurationDoneRequest for debug-adapters that support it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

3 participants