Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Fixes #72 breaking in worker thread does not suspend other threads. #92

Closed
wants to merge 1 commit into from

Conversation

karthiknadig
Copy link
Member

No description provided.

@int19h
Copy link
Contributor

int19h commented Feb 16, 2018

I'm confused - won't this just report incorrect stack traces on those other threads in VS (which will still believe that all threads have stopped)?

@karthiknadig
Copy link
Member Author

karthiknadig commented Feb 16, 2018

In pycharm and VSC, both they show the text/icon that says other threads are running. In VS, it attempts to get a stack for a running thread which is the problem. This fix will cause VS to show thread stack as unknown.

@karthiknadig
Copy link
Member Author

image

@int19h
Copy link
Contributor

int19h commented Feb 16, 2018

VS expects all threads to pause when any thread hits a breakpoint - it's part of their debugger semantics. If we're changing this, then we're deviating from normal behavior (for VS debuggers in general, and for our old debugger in particular), and in a way that is non-obvious for users to distinguish - since VS will tell them that those threads are stopped...

@karthiknadig
Copy link
Member Author

We were testing the old debugger, it continued to print for the above code.

@int19h
Copy link
Contributor

int19h commented Feb 16, 2018

That's really weird - are you sure it wasn't accidentally picking up the new one?

FWIW, the requirement that all threads break is spelled out in VS debug adapter docs:

Visual Studio requires all threads to enter and leave break mode at the same time. Stepping, stopping, or continuing a single thread is not supported.

So I'm kinda hesitant to deviate from that. We should at least ask them what the consequences might be.

@karthiknadig
Copy link
Member Author

I will recall this change as well. Let's figure out the right behavior, as expected by VS and VSC users.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants