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

Resolve "restart" inconsistencies #58817

Closed
weinand opened this issue Sep 17, 2018 · 1 comment
Closed

Resolve "restart" inconsistencies #58817

weinand opened this issue Sep 17, 2018 · 1 comment
Assignees
Labels
debt Code quality issues debug Debug viewlet, configurations, breakpoints, adapter issues

Comments

@weinand
Copy link
Contributor

weinand commented Sep 17, 2018

Currently the debugger implementation has three ways to restart a debug session:

  • EH debugging: new rawDS is created and old rawDS is abandoned; DebugSession stays the same,
  • restart without DAP restartRequest: DebugSession (and its rawDS) is terminated and new DebugSession (with new ID) is created with the heavy "startDebugging" API.
  • restart with DAP restartRequest: DebugSession and rawDS is kept but if the preLaunchTask fails, and user requests to debug anyway a new DebugSession will be created on top of the still existing old DebugSession. This asks for trouble...

We need to fix this by following these principles and suggestions:

  • the DebugSession and its ID must survive the restart. To achieve this we should use the EH Debugging as a model: DebugService.attachExtensionHost shows what needs to be done to reuse the DebugSession. Basically it just replaces the rawDebugSession by a new one.
  • Ideally DebugService.restartSession should become similar to DebugService.attachExtensionHost because they do the same thing (and serve the same UI button).
  • DebugService.runTask is used both for the postTask and the preLaunchTask but its implementation still assumes here and here that it is only meant for the preLaunchTask. runTask should better return a enum value to let its caller know what to do instead of doing it itself.
@weinand weinand added bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues labels Sep 17, 2018
@weinand weinand added this to the On Deck milestone Sep 17, 2018
@isidorn isidorn modified the milestones: On Deck, September 2018 Sep 17, 2018
@isidorn isidorn added the debt Code quality issues label Sep 17, 2018
isidorn added a commit that referenced this issue Sep 21, 2018
@isidorn
Copy link
Contributor

isidorn commented Sep 21, 2018

First of all thanks for this great feedback.

I have cleaned up the restart in the following way:

  • now the session is preserved across restarts in all cases
  • there should be no different behavior between automatic / custom / manual restarts
  • code looks cleaner and there is some minor code share between attach to extension host and restart

@isidorn isidorn removed the bug Issue identified by VS Code Team member as probable bug label Sep 27, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

No branches or pull requests

2 participants