Skip to content

Commit

Permalink
service/dap: include selected goroutine in threads request (#2683)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
suzmue authored Sep 1, 2021
1 parent 7dddcc1 commit 88b163d
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 5 deletions.
39 changes: 34 additions & 5 deletions service/dap/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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:
Expand All @@ -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()

Expand Down
57 changes: 57 additions & 0 deletions service/dap/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 88b163d

Please sign in to comment.