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

debug UI doesn't cleanup spinners on debug session end #80402

Closed
weinand opened this issue Sep 5, 2019 · 6 comments

Comments

@weinand
Copy link
Member

commented Sep 5, 2019

  • modify mock-debug so that the response of the variables request is delayed:
     		setTimeout(() => {
     			this.sendResponse(response);
     		}, 1000 * 100);
  • start debugging
  • observe spinning in Variables view
  • terminate debug session

Observe: spinning continues

@weinand weinand added bug debug labels Sep 5, 2019
@weinand weinand changed the title debug UI doesn't cleanup pending requests on debug session end debug UI doesn't cleanup spinners on debug session end Sep 5, 2019
@isidorn isidorn added this to the September 2019 milestone Sep 5, 2019
@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

I will look into this while we add the cancelation support. It goes in the same direction.

@weinand

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2019

Makes perfect sense. I found the error because I was adding cancelation support to mock debug ;-)

@isidorn isidorn closed this in f17f3f4 Sep 12, 2019
@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

The issue here was that debug adapters running in the EH were not calling cancel pending requests on stop session.
An additional issues was that we were accidently clearing pending requests before actually completing them.

Once I fixed this I found out that Mock debug does not handle very well the stiuation when I cancel a request and after that send a new one.
However I could not reproduce this issue with other adapters so I suspect this is something mock debug specific.

@weinand

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

@isidorn good find with the clearing pending requests before actually completing them.

But you don't have to copy one map into another map by rehashing all the elements.

It is sufficient to just do this:

	const pending = this.pendingRequests;
	this.pendingRequests = new Map<number, (e: DebugProtocol.Response) => void>();
@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

Thanks for the snippet that makes sense. However in this particular case (after some code changes) I want to give 500 ms for the promise to finish on its own, thus it has to be in the this.pendingRequests and I can not clear the whole map right away.

@weinand

This comment has been minimized.

Copy link
Member Author

commented Sep 12, 2019

I see. But why are you then copying the pending requests at all into the "pending" local variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.