Repository: go-delve/delve
Pull request #2683 (2021-08-31 21:00:21)

service/dap: include selected goroutine in threads request
When we set a limit on the number of threads that would be
returned, it was possible that the selected thread was not
included in the list of threads. This could cause issues
because the stopped reason is associated with the selected
goroutine, so users could be missing out on important info.

This change makes sure that the selected goroutine is included
by adding it to the end of the list.


Patch:
diff --git a/service/dap/server.go b/service/dap/server.go
index 3e90c430..b2edbdb3 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -193,7 +193,11 @@ const (
 	// what is presented. A common use case of a call injection is to
 	// stringify complex data conveniently.
 	maxStringLenInCallRetVars = 1 << 10 // 1024
+)
+
+var (
 	// Max number of goroutines that we will return.
+	// This is a var for testing
 	maxGoroutines = 1 << 10
 )
 
@@ -1358,8 +1362,8 @@ func (s *Server) onThreadsRequest(request *dap.ThreadsRequest) {
 	if s.debugger != nil {
 		gs, next, err = s.debugger.Goroutines(0, maxGoroutines)
 	}
-	threads := make([]dap.Thread, len(gs))
 
+	var threads []dap.Thread
 	if err != nil {
 		switch err.(type) {
 		case proc.ErrProcessExited:
@@ -1375,16 +1379,41 @@ func (s *Server) onThreadsRequest(request *dap.ThreadsRequest) {
 				}})
 		}
 		threads = []dap.Thread{{Id: 1, Name: "Dummy"}}
-	} else if len(threads) == 0 {
+	} else if len(gs) == 0 {
 		threads = []dap.Thread{{Id: 1, Name: "Dummy"}}
 	} else {
-		if next >= 0 {
-			s.logToConsole(fmt.Sprintf("Too many goroutines, only loaded %d", len(gs)))
-		}
 		state, err := s.debugger.State( /*nowait*/ true)
 		if err != nil {
 			s.log.Debug("Unable to get debugger state: ", err)
 		}
+
+		if next >= 0 {
+			s.logToConsole(fmt.Sprintf("Too many goroutines, only loaded %d", len(gs)))
+
+			// Make sure the selected goroutine is included in the list of threads
+			// to return.
+			if state != nil && state.SelectedGoroutine != nil {
+				var selectedFound bool
+				for _, g := range gs {
+					if g.ID == state.SelectedGoroutine.ID {
+						selectedFound = true
+						break
+					}
+				}
+				if !selectedFound {
+					g, err := s.debugger.FindGoroutine(state.SelectedGoroutine.ID)
+					if err != nil {
+						s.log.Debug("Error getting selected goroutine: ", err)
+					} else {
+						// TODO(suzmue): Consider putting the selected goroutine at the top.
+						// To be consistent we may want to do this for all threads requests.
+						gs = append(gs, g)
+					}
+				}
+			}
+		}
+
+		threads = make([]dap.Thread, len(gs))
 		s.debugger.LockTarget()
 		defer s.debugger.UnlockTarget()
 


Test patch:
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index 080768a9..5c3deeda 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -922,6 +922,63 @@ func TestStackTraceRequest(t *testing.T) {
 	})
 }
 
+func TestSelectedThreadsRequest(t *testing.T) {
+	runTest(t, "goroutinestackprog", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSessionWithBPs(t, client, "launch",
+			// Launch
+			func() {
+				client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
+			},
+			// Set breakpoints
+			fixture.Source, []int{20},
+			[]onBreakpoint{{
+				execute: func() {
+					checkStop(t, client, 1, "main.main", 20)
+
+					defaultMaxGoroutines := maxGoroutines
+					defer func() { maxGoroutines = defaultMaxGoroutines }()
+
+					maxGoroutines = 1
+					client.SetBreakpointsRequest(fixture.Source, []int{8})
+					client.ExpectSetBreakpointsResponse(t)
+
+					client.ContinueRequest(1)
+					client.ExpectContinueResponse(t)
+
+					se := client.ExpectStoppedEvent(t)
+					if se.Body.Reason != "breakpoint" || se.Body.ThreadId == 1 {
+						t.Errorf("got %#v, want Reason=%q, ThreadId!=1", se, "breakpoint")
+					}
+
+					client.ThreadsRequest()
+					oe := client.ExpectOutputEvent(t)
+					if !strings.HasPrefix(oe.Body.Output, "Too many goroutines") {
+						t.Errorf("got %#v, expected Output=\"Too many goroutines...\"\n", oe)
+
+					}
+					tr := client.ExpectThreadsResponse(t)
+
+					if len(tr.Body.Threads) != 2 {
+						t.Errorf("got %d threads, expected 2\n", len(tr.Body.Threads))
+					}
+
+					var selectedFound bool
+					for _, thread := range tr.Body.Threads {
+						if thread.Id == se.Body.ThreadId {
+							selectedFound = true
+							break
+						}
+					}
+					if !selectedFound {
+						t.Errorf("got %#v, want ThreadId=%d\n", tr.Body.Threads, se.Body.ThreadId)
+					}
+				},
+				disconnect: true,
+			}})
+
+	})
+}
+
 // TestScopesAndVariablesRequests executes to a breakpoint and tests different
 // configurations of 'scopes' and 'variables' requests.
 func TestScopesAndVariablesRequests(t *testing.T) {