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

Debugging shifts focus to another thread, while running, under certain conditions #65920

Closed
segevfiner opened this issue Jan 2, 2019 · 18 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded

Comments

@segevfiner
Copy link
Contributor

  • VSCode Version: 1.30.1
  • OS Version: Windows 10.0.17763.195 x64

Experienced while debugging using vscode-go/delve: microsoft/vscode-go#2133

Steps to Reproduce

  1. Debug the following using vscode-go/delve, placing a breakpoint on the time.Sleep line:
package main

import (
	"fmt"
	"time"
)

func main() {
	for {
		fmt.Println("Hello, World!")
		time.Sleep(2 * time.Second)
	}
}
  1. Constantly hit "next" (step over).
  2. VS Code will often jump to a different thread stopped on gopark while the code is running. Returning to the real debugged thread while stopped.

Analysis

Debugging VS Code a bit, it seems to me we are hitting this line in VS Code: vscode/src/vs/workbench/parts/debug/electron-browser/debugService.ts:496 and that call to focusStackFrame is selecting to focus some other thread than the one currently being debugged, and that thread is, more often than not, stopped on gopark.

We really shouldn't be reaching a code path where VS Code selects some arbitrary thread to focus, that seems like a bug in VS Code.

The code there is a bit weird too, like vscode/src/vs/workbench/parts/debug/electron-browser/debugService.ts:773 with that shift... selecting the second stopped thread...? 😕

I launched VS Code (Git build) with --remote-debugging-port=9222 and vscode-go installed (--extensionDevelopmentPath or just install it), used the "Attach to VS Code" launch configuration in VS Code's repository, and then conditional breakpoints to stop when it's requesting a stackTrace on threadId === 2.

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

cc @ramya-rao-a

@isidorn
Copy link
Contributor

isidorn commented Jan 3, 2019

@segevfiner thanks for this nice investigation.
Currently if you have Thread A and B both stopped. If you focus on A and continue it, B will get auto focussed.
This is the desired behavior for most scenarios IMHO.
I understand this is not desirable for your go scenario, but it makes a lot of sense in other cases.
If there is no workarouind on the GO side for this we could consider to introduce an option to control this, though I am not a big fan of this.

@testforstephen what would be ideal for the Java use case?

@isidorn isidorn added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Jan 3, 2019
@isidorn isidorn added this to the Backlog milestone Jan 3, 2019
@aaronjwood
Copy link

@isidorn are you saying that the functionality before was broken and now it's working as intended? I've been running into this lately and never used to hit it. If it's valid functionality I find that it makes debugging a huge pain, on almost every step over it jumps into proc.go and I have to wait a second or two for it to jump back to where I was. I can see your point where you're breaking inside goroutines and are relying on switching focus. It sounds like there's a bug where the debugger is wrongly picking up on the internal coroutine scheduling.

@segevfiner
Copy link
Contributor Author

I don't think that's really such a desired behavior... Have you ever debugged anything in Visual Studio, windbg, gdb, Eclipse, IntelliJ. and had it jump around to other threads while stepping? I know I haven't. They will only jump to a different thread when the debugee hits a stop event in a different thread, like a breakpoint. It doesn't matter what the other threads are doing, running or stopped. The debugger will keep the debugged thread focused so long as no other thread hits a stop event

@virtuald
Copy link

virtuald commented Jan 4, 2019

@isidorn the scenario you described seems sensible in the case of a continue. However, if A and B are stopped, and I do a step, I would expect to stay in the context of A and not switch to B.

I'm not sure what the difference is (if any) between a step and a continue from the perspective of a debugger, but from a user perspective they definitely seem like different things.

@isidorn isidorn modified the milestones: Backlog, December/January 2019 Jan 4, 2019
@leesjensen
Copy link

@isidorn Maybe I don't understand your response, but the scenario defined by this report does not have two threads stopped. At least not by the user doing the debugging. The user is debugging a single thread and the debugging environment is flashing (temporarily) to another thread. Very annoying. Pretty much like trying to read a web page and every time you read the next sentence it flips to some random page for a second. It makes debugging a painful process. I think the OP did a great job identifying where the problem is happening. Were you able to confirm what was said? Thanks!

@leesjensen
Copy link

@ramya-rao-a You were involved in the initial discussion of this bug

Debugging goes into proc.go at every step #2133

Work on this problem seems to have been dropped. Could you help move the conversation forward? Thanks!

@isidorn
Copy link
Contributor

isidorn commented Jan 9, 2019

The issue is assigned to december/januar which means I plan to investigate more in this milestone.

@ramya-rao-a
Copy link
Contributor

@leesjensen

Work on this problem seems to have been dropped. Could you help move the conversation forward?

That was just the bot closing the issue due to inactivity on the issue. I have re-opened that bug. Work on this problem has not been dropped. It just got moved here :)

@segevfiner has some great findings on the issue and @isidorn will be following up on this.

@isidorn
Copy link
Contributor

isidorn commented Jan 23, 2019

Thanks again to @segevfiner for this investigation and for everybody else for providing feedback.
I was wrong in auto focusing different thread while a thread is running.

Now we only auto focus threads if that particular thread is stopped.
Please try it out in tomorrow's vscode insiders and let me know how it behaves for you.

@testforstephen it would be graet if you also try this out for the java case

isidorn added a commit that referenced this issue Jan 23, 2019
@leesjensen
Copy link

I can confirm that it is working for the Go case.

@isidorn
Copy link
Contributor

isidorn commented Jan 25, 2019

@leesjensen thanks for confirming, adding verified label.
Let me know if you see some regressions

@isidorn isidorn added verified Verification succeeded verification-needed Verification of issue is requested labels Jan 25, 2019
@testforstephen
Copy link

testforstephen commented Feb 12, 2019

@isidorn

Here is the vscode insider version where i tried java multiple threads program. It still has similar switching problem as the issue #55862 when exec continue operation.

Commit: 709bf352b12de8284683bef94cb5984129bfc776
Date: 2019-02-08T14:53:26.555Z
Electron: 3.1.3
Chrome: 66.0.3359.181
Node.js: 10.2.0
V8: 6.6.346.32
OS: Windows_NT x64 10.0.17763

Below is the java program ThreadTest.java i used. Add two breakpoints at line 14 and line 27. When exec next, it doesn't auto switch thread focus, this is expected. But when exec continue, the thread focus always stay on one thread, not auto jump to other suspend threads.

ThreadTest.java
import java.util.concurrent.atomic.AtomicInteger;

public class ThreadTest {
public static void main(String[] args) throws Exception {
Object object = new Object();
AtomicInteger ai = new AtomicInteger();
Thread thread0 = new Thread(() -> {
for (;;) {
System.out.println(String.format("Print %d in thread 0" , ai.incrementAndGet()));
synchronized(object) {
object.notify();
}
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
e.printStackTrace();
break;
}
}
});
Thread thread1 = new Thread(() -> {
for (;;) {
try {
synchronized(object) {
object.wait();
}
System.out.println(String.format("Print %d in thread 1" , ai.incrementAndGet()));
} catch (InterruptedException e) {
e.printStackTrace();
break;
}
}
});

    thread0.start();
    thread1.start();
    thread0.join();
    thread1.join();
}

}

Below are the reproduce steps in vscode-insider:

  1. Add two breakpoints at line 14 and line 27.
  2. Start java debug, there are two breakpoints hit at line 14 (thread-0) and 27 (thread-1).
  3. Select thread-0 to continue, line 14 will be hit again and the focus always stay on thread-0. The focus never switch to thread-1. As comparison, try the same steps both in eclipse and intellij, click continue and it will switch back and forth between thread-0 and thread-1. I think we should maintain a queue to store all suspend threads, every time when exec continue, pop one. And a new breakpoint event arrives, push it.

@testforstephen
Copy link

Have a look at the discussion history. What @segevfiner complains is stepping (next command), agree that it should not auto switch focus, because generally the user wants his focus stay on the current code when stepping. But for continue, it will resume current thread, and sensible to guide the user to the next stopping threads.

@isidorn
Copy link
Contributor

isidorn commented Feb 18, 2019

@testforstephen thanks for your feedback and sorry for the slow response I was on vacation. Can you please create a new issue and ping me on it so we continue the discussion there.
Bottom line: the issue is that the step and continue are handled the same by vscode, and in practice they can produce exactly the same result, so distungishing between them does not always make sense.

@testforstephen
Copy link

@isidorn sure, i will open a new issue to continue the discussion. A quick question here, is there any technical limit that needs keep the step and continue behaving the same? because in eclipse and intellij, step and continue are not the same behavior.

@isidorn
Copy link
Contributor

isidorn commented Feb 18, 2019

There is no technical limit, we could change the behavior in theory.

@testforstephen
Copy link

testforstephen commented Feb 18, 2019

a more comment here. maybe java is a little special because it supports Thread Level suspend, that means you can choose to continue one stopping thread, and keep other threads still on stopping state. Other languages, like nodejs, continue will resume the whole program, so it doesn't need concern the focus switch.

@testforstephen
Copy link

@isidorn Opened a new issue #68955 to continue discussion.

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants
@aaronjwood @virtuald @isidorn @leesjensen @testforstephen @ramya-rao-a @segevfiner and others