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

Can't cancel a debug session while it's initializing #62346

Closed
roblourens opened this Issue Nov 1, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@roblourens
Member

roblourens commented Nov 1, 2018

This seems broken again, maybe since the change to show the debug widget while initializing. Plus, sometimes it doesn't time out after failing to attach, so I'm stuck forever.

Launch this config with nothing running on that port, so it doesn't have anything to attach to:

{
    "type": "node",
    "request": "attach",
    "name": "Attach",
    "port": 9229
},
  • Press F5, debug widget appears
  • Clicking the red stop button or pressing shift+f5 has no effect
  • I can't start another debug config while in this state
  • Sometimes, it times out after 10s. But sometimes while trying this, it never timed out and I had to reload the window in order to debug
@isidorn

This comment has been minimized.

Contributor

isidorn commented Nov 1, 2018

I still can not reproduce this
@eamodio @roblourens can you please do some debugging on your end: put a breakpoint here and figure out if it is nicely being called once you click on stop. If yes check if the sessions array contains the session and if the terminate is nicely being sent out. What node version are you using?

@weinand can you please try to repro this?

@weinand

This comment has been minimized.

Member

weinand commented Nov 1, 2018

@isidorn I can reproduce the problem reliably.
Pressing the red stop button once prevents the timeout (and a reload is needed).
Without pressing the red stop button a timeout appears and everything is fine.
Starting to debug issue...

@weinand

This comment has been minimized.

Member

weinand commented Nov 1, 2018

The issue is (again) a missing call to this.endInitializingState();
I've debugged the whole flow after clicking the red stop button and everything looks correct.
At the end an didEndAdapter event is fired and we end up in

But at the end of the handler a this.endInitializingState(); is missing.

@isidorn please verify that calling this.endInitializingState(); at the end of the handler is the correct thing to do. It fixed the problem for me.

@isidorn

This comment has been minimized.

Contributor

isidorn commented Nov 1, 2018

@weinand yeah calling this.endInitializingState() at the end of that handler makes sense. I will push a fix like that later today.

For debt week I have created this item
#62390

@isidorn isidorn closed this in 220bff2 Nov 1, 2018

@isidorn

This comment has been minimized.

Contributor

isidorn commented Nov 1, 2018

Pushed a workaround that I end the initialising state on adapter exit. This should fix it and can not have any side effects (since if we are not initialising it is a no-op).
The actual fix is assigned for debt week as mentioned in my previous comment.

@weinand weinand added debug verified and removed needs more info labels Nov 2, 2018

@eamodio

This comment has been minimized.

Contributor

eamodio commented Nov 2, 2018

@isidorn it seems to work most of the time now, but sometimes if you are quick you have to hit the stop button more than once to get it to work, and sometimes it results in this dialog:

image

Also related -- if there is already a debugging session going (say debugging an extension) and then you start an attach (to say a language server) the attach won't show up in toolbar until the attach succeeds. Although if you hit the stop button it will stop the extension debugging and then the toolbar will show the pending attach operation, which can be canceled.

@isidorn

This comment has been minimized.

Contributor

isidorn commented Nov 5, 2018

@eamodio I plan to do some refactoring in this week which might improve this. If you still see those issues in a week it would be great to open a new issue so we can work on fixing those. Thanks a lot

@eamodio

This comment has been minimized.

Contributor

eamodio commented Nov 21, 2018

@isidorn I am still seeing all of the issues above in #62346 (comment). Should I open a couple new issues?

@isidorn

This comment has been minimized.

Contributor

isidorn commented Nov 21, 2018

@eamodio yes please open a new issue, thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.