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

Respect the CancelationToken in provideDebugConfigurations and resolveDebugConfigurations to cancel debugging #77293

Closed
ivanhernandez13 opened this issue Jul 12, 2019 · 9 comments
Assignees
Labels
debt Code quality issues debug Debug viewlet, configurations, breakpoints, adapter issues
Milestone

Comments

@ivanhernandez13
Copy link

ivanhernandez13 commented Jul 12, 2019

  • VSCode Version: 1.36 & 1.37-insiders
  • OS Version: macOS 10.14.5

Steps to Reproduce:

  1. clone vscode-mock-debug.
  2. Open vscode-mock-debug in vscode.
  3. Add the following to resolveDebugConfiguration() in extension.ts.
    for (let i = 0; i < 10000; i++) { console.log(i) }
  4. Start debugging the extension.
  5. Open a workspace with a MARKDOWN file.
  6. Start a debug session and immediately click on stop.

The debug toolbar will hide but won't do anything. The debug adapter will still launch and the debug session will begin and thus the toolbar will reappear.

Video: https://1drv.ms/v/s!AiguEN65RhivhFQM-7S7ayzDQ3l6

Note in the video that clicking on stop before the debug adapter has launched won't trigger vscode.debug.onDidTerminateDebugSession() to be called. Is there a way to detect that the user has tried to stop the debug session after they have clicked to start the debug session but before the debug adapter has launched e.g. in the middle of resolveDebugConfiguration()? resolveDebugConfiguration() takes a CancellationToken but adding an event handler to onCancellationRequested() doesn't seem to be called and checking isCancellationRequested at the end of resolveDebugConfiguration() is still false.

Does this issue occur when all extensions are disabled?: Yes

@weinand weinand assigned isidorn and unassigned weinand Jul 12, 2019
@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jul 12, 2019
@isidorn
Copy link
Contributor

isidorn commented Jul 30, 2019

Well the stop sent a disconnect request to the debug extension. HOwever the debug extension ignored it and still started.
So the debug extensions should get a disconnect request. It can respect it and stop starting.

@ivanhernandez13 are you an extension writer. Do you see some idfferent behavior than what I just mentioned?

And sorry for the slow reponse, I was on vacation.

@isidorn isidorn added the info-needed Issue requires more information from poster label Jul 30, 2019
@ivanhernandez13
Copy link
Author

No worries, thanks for the response. I am working on a debug extension that uses an executable for the debug adapter.

The series of events you describe are accurate but only occur after the debug adapter has started and that only occurs after resolveDebugConfiguration() has completed. The order of events I am seeing is:

  1. Click on 'Start Debugging'
  2. resolveDebugConfiguration() is called and performs its duties.
  3. resolveDebugConfiguration() completes and returns a complete debug configuration.
  4. createDebugAdapterDescriptor() is called and returns a vscode.DebugAdapterExecutable or vscode.DebugAdapterServer.
  5. Debug adapter executable is spawned or connected to.
  6. Debug adapter receives initialize request. (Can also receive disconnect requests)

So if the user happens to click stop between steps 2-5, there is no way to detect a disconnect request (that I can see). In the example and video I provided, I happen to click on stop while in step 2 so the disconnect request can't be caught yet because the adapter hasn't been spawned. Or am I missing something and there is a way in the extension itself to detect that stop was clicked on?

I mentioned I tried the cancellation token's onCancellationRequested() and vscode.debug.onDidTerminateDebugSession() events but neither were called.

@isidorn
Copy link
Contributor

isidorn commented Aug 5, 2019

Ok, thanks for providing more details.
Yes you are right it can happen that the disconnect comes before the initialize.

In order to reproduce this situation I have changed the initialize to be sent with a timeout.
That way I would get into a situation that the disconnect happens first. And I could still see the vscode.debug.onDidTerminateDebugSession() being fired.
So that event should always get fired, even if the session is not fully initialised when it is shutdown by the user.

If you would like me to investigate more, please create a fork of mock-debug with the changes such that I can just clone it and reproduce the issue on my machine such that the onDidTerminateDebugSession is not fired.
Thanks!

@ivanhernandez13
Copy link
Author

https://github.com/ivanhernandez13/vscode-mock-debug.git

This will wait 5 seconds before resolving the debug configuration in resolveDebugConfiguration(). It will also present a message when the debug session is terminated. If you hit stop before those 5 seconds nothing will happen.

@isidorn
Copy link
Contributor

isidorn commented Aug 6, 2019

@ivanhernandez13 Great steps. I can now reproduce the issue nicely and I acknowledge what you were saying: that the debug adapter is not yet spawned, thus it is not possible to send a disconnect.
The only way to fix this is to propagate the cancelation token form our API down to our internal debug logic.
@weinand assigning to you to look into propagating the cancelation token down, and then I can wire it in.

@isidorn isidorn removed the info-needed Issue requires more information from poster label Aug 6, 2019
@weinand
Copy link
Contributor

weinand commented Aug 6, 2019

@isidorn the two ConfigurationManager methods resolveConfigurationByProviders and
provideDebugConfigurations now have an optional CancellationToken argument.

@isidorn
Copy link
Contributor

isidorn commented Aug 6, 2019

@weinand thanks.
I have wired the cancelation token in and it seems to work nicely.
@ivanhernandez13 I have tried with your sample and it looks good. You can also verify once we release new insiders in a couple of days.
Basicaly now when the user stops the initialisaition of the debug session the cancelation token will get cancled.

@isidorn isidorn added the debt Code quality issues label Aug 6, 2019
@isidorn isidorn added this to the August 2019 milestone Aug 6, 2019
@isidorn isidorn closed this as completed in 5cad6c4 Aug 6, 2019
@ivanhernandez13
Copy link
Author

I built VS Code from head to try this out and works great, thanks!

@isidorn
Copy link
Contributor

isidorn commented Aug 8, 2019

@ivanhernandez13 thanks for letting us know!

@isidorn isidorn changed the title Clicking on stop icon in the debug toolbar doesn't do anything until the debug adapter is launched. Respect the CancelationToken in provideDebugConfigurations and resolveDebugConfigurations to cancel debugging Sep 4, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 20, 2019
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

3 participants