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

Display debugged sub-processes as a treeview #62972

Closed
DonJayamanne opened this issue Nov 12, 2018 · 21 comments
Closed

Display debugged sub-processes as a treeview #62972

DonJayamanne opened this issue Nov 12, 2018 · 21 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality release-notes Release notes issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@DonJayamanne
Copy link
Contributor

Terms

  • sub-process: It’s basically a process started by a parent process (analogous to spawn in node).

Debug toolbar should show run configurations in drop-down, not processes

  • The current solution implemented within the python extension for sub-process debugging is as follows:
    • User launches some python code in VSC debugger
    • The user code in the python program then spins up a sub process
    • The Python debug adapter intercepts the above call, and launches a sub-process in a remote debugging manner (so that the VS Code debugger can attach to it).
    • The extension gets a notification about this with the new process ID and starts a debug session by attaching to the above sub-process
    • We now have a dropdown in the debug toolbar with two items:
    • First item – Original debug configuration name
    • Second item – “Child Process xyz” (name provided by the extension when starting the new debug session)

Expected behavior

  • VSC needs to provide a way to display child processes in the UI.
  • Single click to stop debugging (when debugging child processes)

Current behavior

  • Python Extension is displaying child process as another debug configuration.
  • This leads to confusions amongst users.
    • The user launched one program for debugging, why are they now getting two debug sessions, with a stop and detach button?
    • The user has to click on the red icon to stop debugging twice (first Detach then Stop).
  • If the user were to start debugging some other application such as React/node.js (or similar), then this additional item in the dropdown of the debug toolbar adds to the confusion.
    • As before, the user started debugging two applications, but they have 3 items in the dropdown.
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality labels Nov 13, 2018
@weinand weinand added this to the Backlog milestone Nov 13, 2018
@microsoft microsoft deleted a comment from vscodebot bot Nov 13, 2018
@qubitron
Copy link

I wanted to add a few more details using an example.

Currently if I start two different debug configurations (one flask, one django) with auto-reload enabled, I see the following in the debug dropdown:
image

... and the following in the call stack window:
image

What we're saying is that the sub-processes automatically spawned by the parent process should be treated logically as part of one application, but associated with the debug configuration.

So for this example we would be proposing :

  • The debug dropdown only lists Flask and Django (debug configurations I have explicitly launched), the call stack window already lists all processes so they don't need to be repeated in the debug dropdown.
  • The stop button would stop a debug configuration and all of its sub processes automatically, in our case it can simply stop the parent process and then we would clean up sub processes
  • The pause button would pause all processes in that debug configuration
  • The call stack window can continue to show all running processes for all debug configurations, but perhaps if there are multiple processes per debug configuration we could show the relationship as a tree structure, e.g.:
- Django
   - Parent Process
   - Sub Process
- Flask
   - Parent Process
   - Sub Process

@weinand
Copy link
Contributor

weinand commented Nov 14, 2018

Clarification:
The dropdown menu in the debug toolbar shows "debug sessions" not "debug configurations".

Today both VS Code and the Debug Adapter Protocol (DAP) knows nothing about child processes. All functionality supported in the scenarios from above works either automatically or through a small amount of glue code in the debugger extension.

Making VS Code's debugger multi-process aware would be a big effort which not only affects VS Code but the DAP as well. Currently we do not see a big need for this and consequently it is not on our roadmap.

So the only feature that I could imagine to offer for VS Code is some sort of "visual grouping mechanism". So when creating new debug sessions via the extension API (e.g. "startDebugging" call), we could support to pass in a parent debug session and VS Code would honour that by indenting the debug sessions to form a tree.

As a workaround for now you could just prepend some non-breakable whitespace characters (\u00A0) to the generated debug configuration name. I tried this for the node.js Cluster and it looks like this:

2018-11-14_17-18-40

/cc @isidorn

@qubitron
Copy link

qubitron commented Jan 3, 2019

Coming back to this, we were blocked on another issue on our side but will soon be able to make progress.

I don't think we need the DAP to be multi-process aware, if VS Code could be made aware of "Child Debug Sessions" on create as you proposed, what we'd be asking for in the UI would be to:

  • Not show child debug sessions in the debug toolbar dropdown, only the parent
  • Stop debugging on the parent would stop the child debug sessions as well

So in the typical case when you start debugging once, but the app spawns multiple child debug sessions, the user would have a single stop button that would stop all child sessions.

Visual indenting in the call stack window as you've shown would be nice as well, but this is not essential as it would only be for the less common case when the user has intentionally started debugging two different parent debug sessions.

@weinand weinand modified the milestones: Backlog, March 2019 Feb 22, 2019
@weinand
Copy link
Contributor

weinand commented Feb 22, 2019

@qubitron we are planning to implement the "visual grouping of child processes" approach in the next milestone. The only API change will add an additional parent: DebugSession argument to the startDebugging API.

Now I have a question regarding the UI that we will have to implement for this feature.

Above you said:

"Not show child debug sessions in the debug toolbar dropdown, only the parent".

I do not understand how that would work:
If I'm stopped in a python application that has one parent and two child processes and all of them are stopped, how would I single step in one of the child processes, if I'm not able to select the child in the dropdown (because it only shows the parent)?

@qubitron
Copy link

@weinand I would have assumed that clicking on the child process in the call stack window would allow you to switch between active contexts and single step. Though I'm not exactly sure how to get VS Code into multi-proc debugging mode to test if this works.

If it is problematic to hide the child processes from the debug toolbar, the important capability is to make make "stop" stop all of the processes.

In our scenarios most people don't realize multiple processes are in play, they just start debugging and as an implementation detail the web server spawns sub-processes.

@weinand
Copy link
Contributor

weinand commented Mar 21, 2019

@qubitron @DonJayamanne
The latest Insiders supports hierarchical debug sessions (and the proposed API will be promoted to stable next Monday (March 25)).
Please give it a try when using the vscode.debug.startDebugging API.

I'm not yet convinced that making the "Stop action stop all of the processes" is the right thing to do for VS Code. WE can continue the discussion in April.

@weinand weinand modified the milestones: March 2019, April 2019 Mar 21, 2019
@DonJayamanne
Copy link
Contributor Author

DonJayamanne commented Apr 2, 2019

@weinand
Thanks, just tried this and it doesn't seem to work for us.
Here's what I've done:

  • Enabled "enableProposedApi": true,
  • Added the property parent when invoking startDebugging
  • No changes in the UI

Screen Shot 2019-04-02 at 8 48 31 AM

This is when we launch the sub-process.
Screen Shot 2019-04-02 at 8 49 02 AM
Screen Shot 2019-04-02 at 8 48 20 AM
Screen Shot 2019-04-02 at 8 48 40 AM

@weinand
Copy link
Contributor

weinand commented Apr 2, 2019

@DonJayamanne no, the new API is not part of a config but an additional parameter of the startDebugging API:

/**
 * Start debugging by using either a named launch or named compound configuration,
 * or by directly passing a [DebugConfiguration](#DebugConfiguration).
 * The named configurations are looked up in '.vscode/launch.json' found in the given folder.
 * Before debugging starts, all unsaved files are saved and the launch configurations are brought up-to-date.
 * Folder specific variables used in the configuration (e.g. '${workspaceFolder}') are resolved against the given folder.
 * @param folder The [workspace folder](#WorkspaceFolder) for looking up named configurations and resolving variables or `undefined` for a non-folder setup.
 * @param nameOrConfiguration Either the name of a debug or compound configuration or a [DebugConfiguration](#DebugConfiguration) object.
 * @param parent If specified the newly created debug session is registered as a "child" session of a "parent" debug session.
 * @return A thenable that resolves when debugging could be successfully started.
 */
export function startDebugging(folder: WorkspaceFolder | undefined, nameOrConfiguration: string | DebugConfiguration, parentSession?: DebugSession): Thenable<boolean>;

Just set your "vscode" engine version to 1.33 in the package.json and do a "npm install".
Then you should see the API in vscode.d.ts.
You don't have to use the "enableProposedApi": true.
And your code will still work with older versions of VS Code (because the parentSession is just ignored).

@weinand
Copy link
Contributor

weinand commented Apr 2, 2019

@qubitron @DonJayamanne @isidorn

Proposal resulting from today's discussion:

  • we do not show child sessions in the debug drop down,
  • in the debug tool bar we do not install the stop action for the active debug session but install the stop session of its parent instead.
  • in the Call Stack viewer we continue to show debug sessions as a tree.
  • Individual sessions can be terminated via the context menu in the Call Stack viewer.

Compound launch configs are not affected by this.
Cluster debugging is affected as child sessions are no longer available in the drop down menu.

Whether this behavior is configurable at all, or as a user setting or as an additional option passed to "startDebugging" has to be decided.

@weinand
Copy link
Contributor

weinand commented Apr 3, 2019

@isidorn could you please implement the proposal behind an experimental setting.

@isidorn
Copy link
Contributor

isidorn commented Apr 3, 2019

@weinand yes. Let me look into this.

isidorn added a commit that referenced this issue Apr 3, 2019
@isidorn
Copy link
Contributor

isidorn commented Apr 3, 2019

I have pushed the experimental setting
debug.hideSubSessions which when set to true will make us no longer show sub sessions in the debug drop down. Also this flag affects the behavior of the stop action.
This flag by default is false.

In both cases the individual sub-sessions can be terminated via context menu in the call stack viewer.

Try it out and let me know what you think.

After getting some feedback as @weinand already pointed out we should figure out if this should be configurable or not. If yes we should find a better name for the setting probably.

@DonJayamanne
Copy link
Contributor Author

Will try in tomorrows insider build. Thanks.

@DonJayamanne
Copy link
Contributor Author

DonJayamanne commented Apr 5, 2019

Tried and works great.
Hopefully we can get this turned into a debug configuration item & not a debug setting item (as is done today for experimenting with).

@qubitron /cc

Screen Shot 2019-04-04 at 8 53 47 PM

@weinand
Copy link
Contributor

weinand commented Apr 9, 2019

After using this setting for some time I propose to make this setting official:

  • rename debug.hideSubSessions to debug.showSubSessionsInToolBar
  • make false the default value
  • add Intellisense info for it

@DonJayamanne @testforstephen @roblourens any objections?

@testforstephen
Copy link

I like this feature because we meet similar issues before during reusing java debugger for other extensions.

But one question here: if the sub debug session is hided in the debug toolbar, how to step in the sub debug session? Clicking the call stack to switch the active debug session?

@weinand
Copy link
Contributor

weinand commented Apr 10, 2019

@testforstephen yes, the call stack is the preferred way to switch between sessions, processes, and threads.

@weinand
Copy link
Contributor

weinand commented Apr 10, 2019

One important thing to note: this new behavior only affects hierarchical debug session (that use the new vscode.dbug.startDebugging API).
If debug sessions are started independently, they will still show up as two sessions in the drop down in the debug toolbar.
So existing debug extensions that do not use that API will see no change.

@isidorn
Copy link
Contributor

isidorn commented Apr 10, 2019

I have introduced the debug.showSubSessionsInToolBar
Here's the setting description I came up with. Feedback welcome
Controls whether the debug sub-sessions are shown in the debug tool bar. When this setting is false the stop command on a sub-session will also stop the parent session.

Try it out and let me know how it behaves.

@DonJayamanne
Copy link
Contributor Author

Thanks, we're happy with the changes, apologies for the delay in getting back.

@isidorn isidorn added verification-needed Verification of issue is requested verified Verification succeeded labels May 8, 2019
@isidorn
Copy link
Contributor

isidorn commented May 8, 2019

Adding verified label per @DonJayamanne comment

@isidorn isidorn added the release-notes Release notes issues label May 9, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators May 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality release-notes Release notes issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants