Skip to content

Commit 89ed5a0

Browse files
suzmuesuzmue
authored
service/dap: page stack frames (#2597)
Returning monotonically increasing totalFrames values for subsequent requests can be used to enforce paging in the client. If we are not at the end of the stackFrames that are returned, we return a higher total frames to suggest to the client that they should request more frames.
1 parent b87a1fc commit 89ed5a0

File tree

2 files changed

+147
-51
lines changed

2 files changed

+147
-51
lines changed

‎service/dap/server.go

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1676,7 +1676,19 @@ func (s *Server) onStackTraceRequest(request *dap.StackTraceRequest) {
16761676
}
16771677

16781678
goroutineID := request.Arguments.ThreadId
1679-
frames, err := s.debugger.Stacktrace(goroutineID, s.args.stackTraceDepth, 0)
1679+
start := request.Arguments.StartFrame
1680+
if start < 0 {
1681+
start = 0
1682+
}
1683+
levels := s.args.stackTraceDepth
1684+
if request.Arguments.Levels > 0 {
1685+
levels = request.Arguments.Levels
1686+
}
1687+
1688+
// Since the backend doesn't support paging, we load all frames up to
1689+
// the requested depth and then slice them here per
1690+
// `supportsDelayedStackTraceLoading` capability.
1691+
frames, err := s.debugger.Stacktrace(goroutineID, start+levels-1, 0)
16801692
if err != nil {
16811693
s.sendErrorResponse(request.Request, UnableToProduceStackTrace, "Unable to produce stack trace", err.Error())
16821694
return
@@ -1688,36 +1700,35 @@ func (s *Server) onStackTraceRequest(request *dap.StackTraceRequest) {
16881700
isSystemGoroutine = g.System(s.debugger.Target())
16891701
}
16901702

1691-
stackFrames := make([]dap.StackFrame, len(frames))
1692-
for i, frame := range frames {
1703+
stackFrames := []dap.StackFrame{} // initialize to empty, since nil is not an accepted response.
1704+
for i := 0; i < levels && i+start < len(frames); i++ {
1705+
frame := frames[start+i]
16931706
loc := &frame.Call
1694-
uniqueStackFrameID := s.stackFrameHandles.create(stackFrame{goroutineID, i})
1695-
stackFrames[i] = dap.StackFrame{Id: uniqueStackFrameID, Line: loc.Line, Name: fnName(loc)}
1707+
uniqueStackFrameID := s.stackFrameHandles.create(stackFrame{goroutineID, start + i})
1708+
stackFrame := dap.StackFrame{Id: uniqueStackFrameID, Line: loc.Line, Name: fnName(loc)}
16961709
if loc.File != "<autogenerated>" {
16971710
clientPath := s.toClientPath(loc.File)
1698-
stackFrames[i].Source = dap.Source{Name: filepath.Base(clientPath), Path: clientPath}
1711+
stackFrame.Source = dap.Source{Name: filepath.Base(clientPath), Path: clientPath}
16991712
}
1700-
stackFrames[i].Column = 0
1713+
stackFrame.Column = 0
17011714

17021715
packageName := fnPackageName(loc)
17031716
if !isSystemGoroutine && packageName == "runtime" {
1704-
stackFrames[i].Source.PresentationHint = "deemphasize"
1717+
stackFrame.Source.PresentationHint = "deemphasize"
17051718
}
1719+
stackFrames = append(stackFrames, stackFrame)
17061720
}
1707-
// Since the backend doesn't support paging, we load all frames up to
1708-
// pre-configured depth every time and then slice them here per
1709-
// `supportsDelayedStackTraceLoading` capability.
1710-
// TODO(polina): consider optimizing this, so subsequent stack requests
1711-
// slice already loaded frames and handles instead of reloading every time.
1712-
if request.Arguments.StartFrame > 0 {
1713-
stackFrames = stackFrames[min(request.Arguments.StartFrame, len(stackFrames)):]
1714-
}
1715-
if request.Arguments.Levels > 0 {
1716-
stackFrames = stackFrames[:min(request.Arguments.Levels, len(stackFrames))]
1721+
1722+
totalFrames := len(frames)
1723+
if len(frames) >= start+levels && !frames[len(frames)-1].Bottom {
1724+
// We don't know the exact number of available stack frames, so
1725+
// add an arbitrary number so the client knows to request additional
1726+
// frames.
1727+
totalFrames += s.args.stackTraceDepth
17171728
}
17181729
response := &dap.StackTraceResponse{
17191730
Response: *newResponse(request.Request),
1720-
Body: dap.StackTraceResponseBody{StackFrames: stackFrames, TotalFrames: len(frames)},
1731+
Body: dap.StackTraceResponseBody{StackFrames: stackFrames, TotalFrames: totalFrames},
17211732
}
17221733
s.send(response)
17231734
}

‎service/dap/server_test.go

Lines changed: 117 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -597,24 +597,32 @@ func TestPreSetBreakpoint(t *testing.T) {
597597
})
598598
}
599599

600-
// checkStackFrames is a helper for verifying the values within StackTraceResponse.
600+
// checkStackFramesExact is a helper for verifying the values within StackTraceResponse.
601601
// wantStartName - name of the first returned frame (ignored if "")
602602
// wantStartLine - file line of the first returned frame (ignored if <0).
603603
// wantStartID - id of the first frame returned (ignored if wantFrames is 0).
604604
// wantFrames - number of frames returned (length of StackTraceResponse.Body.StackFrames array).
605605
// wantTotalFrames - total number of stack frames available (StackTraceResponse.Body.TotalFrames).
606-
func checkStackFrames(t *testing.T, got *dap.StackTraceResponse,
606+
func checkStackFramesExact(t *testing.T, got *dap.StackTraceResponse,
607607
wantStartName string, wantStartLine, wantStartID, wantFrames, wantTotalFrames int) {
608608
t.Helper()
609-
checkStackFramesNamed("", t, got, wantStartName, wantStartLine, wantStartID, wantFrames, wantTotalFrames)
609+
checkStackFramesNamed("", t, got, wantStartName, wantStartLine, wantStartID, wantFrames, wantTotalFrames, true)
610610
}
611611

612-
func checkStackFramesNamed(testName string, t *testing.T, got *dap.StackTraceResponse,
612+
func checkStackFramesHasMore(t *testing.T, got *dap.StackTraceResponse,
613613
wantStartName string, wantStartLine, wantStartID, wantFrames, wantTotalFrames int) {
614614
t.Helper()
615-
if got.Body.TotalFrames != wantTotalFrames {
615+
checkStackFramesNamed("", t, got, wantStartName, wantStartLine, wantStartID, wantFrames, wantTotalFrames, false)
616+
}
617+
func checkStackFramesNamed(testName string, t *testing.T, got *dap.StackTraceResponse,
618+
wantStartName string, wantStartLine, wantStartID, wantFrames, wantTotalFrames int, totalExact bool) {
619+
t.Helper()
620+
if totalExact && got.Body.TotalFrames != wantTotalFrames {
616621
t.Errorf("%s\ngot %#v\nwant TotalFrames=%d", testName, got.Body.TotalFrames, wantTotalFrames)
622+
} else if !totalExact && got.Body.TotalFrames < wantTotalFrames {
623+
t.Errorf("%s\ngot %#v\nwant TotalFrames>=%d", testName, got.Body.TotalFrames, wantTotalFrames)
617624
}
625+
618626
if len(got.Body.StackFrames) != wantFrames {
619627
t.Errorf("%s\ngot len(StackFrames)=%d\nwant %d", testName, len(got.Body.StackFrames), wantFrames)
620628
} else {
@@ -812,10 +820,9 @@ func TestStackTraceRequest(t *testing.T) {
812820
// repeated requests at the same breakpoint
813821
// would assign next block of unique ids to them each time.
814822
const NumFrames = 6
815-
reqIndex := -1
816-
frameID := func(frameIndex int) int {
817-
reqIndex++
818-
return startHandle + NumFrames*reqIndex + frameIndex
823+
reqIndex := 0
824+
frameID := func() int {
825+
return startHandle + reqIndex
819826
}
820827

821828
tests := map[string]struct {
@@ -826,24 +833,102 @@ func TestStackTraceRequest(t *testing.T) {
826833
wantStartFrame int
827834
wantFramesReturned int
828835
wantFramesAvailable int
836+
exact bool
829837
}{
830-
"all frame levels from 0 to NumFrames": {0, NumFrames, "main.Increment", 8, 0, NumFrames, NumFrames},
831-
"subset of frames from 1 to -1": {1, NumFrames - 1, "main.Increment", 11, 1, NumFrames - 1, NumFrames},
832-
"load stack in pages: first half": {0, NumFrames / 2, "main.Increment", 8, 0, NumFrames / 2, NumFrames},
833-
"load stack in pages: second half": {NumFrames / 2, NumFrames, "main.main", 17, NumFrames / 2, NumFrames / 2, NumFrames},
834-
"zero levels means all levels": {0, 0, "main.Increment", 8, 0, NumFrames, NumFrames},
835-
"zero levels means all remaining levels": {NumFrames / 2, 0, "main.main", 17, NumFrames / 2, NumFrames / 2, NumFrames},
836-
"negative levels treated as 0 (all)": {0, -10, "main.Increment", 8, 0, NumFrames, NumFrames},
837-
"OOB levels is capped at available len": {0, NumFrames + 1, "main.Increment", 8, 0, NumFrames, NumFrames},
838-
"OOB levels is capped at available len 1": {1, NumFrames + 1, "main.Increment", 11, 1, NumFrames - 1, NumFrames},
839-
"negative startFrame treated as 0": {-10, 0, "main.Increment", 8, 0, NumFrames, NumFrames},
840-
"OOB startFrame returns empty trace": {NumFrames, 0, "main.Increment", -1, -1, 0, NumFrames},
838+
"all frame levels from 0 to NumFrames": {0, NumFrames, "main.Increment", 8, 0, NumFrames, NumFrames, true},
839+
"subset of frames from 1 to -1": {1, NumFrames - 1, "main.Increment", 11, 1, NumFrames - 1, NumFrames, true},
840+
"load stack in pages: first half": {0, NumFrames / 2, "main.Increment", 8, 0, NumFrames / 2, NumFrames, false},
841+
"load stack in pages: second half": {NumFrames / 2, NumFrames, "main.main", 17, NumFrames / 2, NumFrames / 2, NumFrames, true},
842+
"zero levels means all levels": {0, 0, "main.Increment", 8, 0, NumFrames, NumFrames, true},
843+
"zero levels means all remaining levels": {NumFrames / 2, 0, "main.main", 17, NumFrames / 2, NumFrames / 2, NumFrames, true},
844+
"negative levels treated as 0 (all)": {0, -10, "main.Increment", 8, 0, NumFrames, NumFrames, true},
845+
"OOB levels is capped at available len": {0, NumFrames + 1, "main.Increment", 8, 0, NumFrames, NumFrames, true},
846+
"OOB levels is capped at available len 1": {1, NumFrames + 1, "main.Increment", 11, 1, NumFrames - 1, NumFrames, true},
847+
"negative startFrame treated as 0": {-10, 0, "main.Increment", 8, 0, NumFrames, NumFrames, true},
848+
"OOB startFrame returns empty trace": {NumFrames, 0, "main.Increment", -1, -1, 0, NumFrames, true},
841849
}
842850
for name, tc := range tests {
843851
client.StackTraceRequest(1, tc.startFrame, tc.levels)
844852
stResp = client.ExpectStackTraceResponse(t)
845853
checkStackFramesNamed(name, t, stResp,
846-
tc.wantStartName, tc.wantStartLine, frameID(tc.wantStartFrame), tc.wantFramesReturned, tc.wantFramesAvailable)
854+
tc.wantStartName, tc.wantStartLine, frameID(), tc.wantFramesReturned, tc.wantFramesAvailable, tc.exact)
855+
reqIndex += len(stResp.Body.StackFrames)
856+
}
857+
},
858+
disconnect: false,
859+
}, {
860+
// Stop at line 18
861+
execute: func() {
862+
// Frame ids get reset at each breakpoint.
863+
client.StackTraceRequest(1, 0, 0)
864+
stResp = client.ExpectStackTraceResponse(t)
865+
checkStackFramesExact(t, stResp, "main.main", 18, startHandle, 3, 3)
866+
},
867+
disconnect: false,
868+
}})
869+
})
870+
runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
871+
var stResp *dap.StackTraceResponse
872+
runDebugSessionWithBPs(t, client, "launch",
873+
// Launch
874+
func() {
875+
client.LaunchRequest("exec", fixture.Path, !stopOnEntry)
876+
},
877+
// Set breakpoints
878+
fixture.Source, []int{8, 18},
879+
[]onBreakpoint{{
880+
// Stop at line 8
881+
execute: func() {
882+
// Even though the stack frames do not change,
883+
// repeated requests at the same breakpoint
884+
// would assign next block of unique ids to them each time.
885+
const NumFrames = 6
886+
887+
var frames []dap.StackFrame
888+
889+
for start, levels := 0, 1; start < NumFrames; {
890+
client.StackTraceRequest(1, start, levels)
891+
stResp = client.ExpectStackTraceResponse(t)
892+
frames = append(frames, stResp.Body.StackFrames...)
893+
if stResp.Body.TotalFrames < NumFrames {
894+
t.Errorf("got %#v\nwant TotalFrames>=%d\n", stResp.Body.TotalFrames, NumFrames)
895+
}
896+
897+
if len(stResp.Body.StackFrames) < levels {
898+
t.Errorf("got len(StackFrames)=%d\nwant >=%d\n", len(stResp.Body.StackFrames), levels)
899+
}
900+
901+
start += len(stResp.Body.StackFrames)
902+
}
903+
904+
// TODO check all the frames.
905+
want := []struct {
906+
wantName string
907+
wantLine int
908+
}{
909+
{"main.Increment", 8},
910+
{"main.Increment", 11},
911+
{"main.Increment", 11},
912+
{"main.main", 17},
913+
{"runtime.main", 0},
914+
{"runtime.goexit", 0},
915+
}
916+
for i, frame := range frames {
917+
frameId := startHandle + i
918+
if frame.Id != frameId {
919+
t.Errorf("got %#v\nwant Id=%d\n", frame, frameId)
920+
}
921+
922+
// Verify the name and line corresponding to the first returned frame (if any).
923+
// This is useful when the first frame is the frame corresponding to the breakpoint at
924+
// a predefined line. Line values < 0 are a signal to skip the check (which can be useful
925+
// for frames in the third-party code, where we do not control the lines).
926+
if want[i].wantLine > 0 && frame.Line != want[i].wantLine {
927+
t.Errorf("got Line=%d\nwant %d\n", frame.Line, want[i].wantLine)
928+
}
929+
if want[i].wantName != "" && frame.Name != want[i].wantName {
930+
t.Errorf("got Name=%s\nwant %s\n", frame.Name, want[i].wantName)
931+
}
847932
}
848933
},
849934
disconnect: false,
@@ -853,7 +938,7 @@ func TestStackTraceRequest(t *testing.T) {
853938
// Frame ids get reset at each breakpoint.
854939
client.StackTraceRequest(1, 0, 0)
855940
stResp = client.ExpectStackTraceResponse(t)
856-
checkStackFrames(t, stResp, "main.main", 18, startHandle, 3, 3)
941+
checkStackFramesExact(t, stResp, "main.main", 18, startHandle, 3, 3)
857942
},
858943
disconnect: false,
859944
}})
@@ -887,7 +972,7 @@ func TestScopesAndVariablesRequests(t *testing.T) {
887972
startLineno = -1
888973
}
889974

890-
checkStackFrames(t, stack, "main.foobar", startLineno, 1000, 4, 4)
975+
checkStackFramesExact(t, stack, "main.foobar", startLineno, 1000, 4, 4)
891976

892977
client.ScopesRequest(1000)
893978
scopes := client.ExpectScopesResponse(t)
@@ -1101,7 +1186,7 @@ func TestScopesAndVariablesRequests(t *testing.T) {
11011186
// Frame ids get reset at each breakpoint.
11021187
client.StackTraceRequest(1, 0, 20)
11031188
stack := client.ExpectStackTraceResponse(t)
1104-
checkStackFrames(t, stack, "main.barfoo", 27, 1000, 5, 5)
1189+
checkStackFramesExact(t, stack, "main.barfoo", 27, 1000, 5, 5)
11051190

11061191
client.ScopesRequest(1000)
11071192
scopes := client.ExpectScopesResponse(t)
@@ -1154,7 +1239,7 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
11541239
execute: func() {
11551240
client.StackTraceRequest(1, 0, 20)
11561241
stack := client.ExpectStackTraceResponse(t)
1157-
checkStackFrames(t, stack, "main.main", -1, 1000, 3, 3)
1242+
checkStackFramesExact(t, stack, "main.main", -1, 1000, 3, 3)
11581243

11591244
client.ScopesRequest(1000)
11601245
scopes := client.ExpectScopesResponse(t)
@@ -1166,7 +1251,7 @@ func TestScopesAndVariablesRequests2(t *testing.T) {
11661251
execute: func() {
11671252
client.StackTraceRequest(1, 0, 20)
11681253
stack := client.ExpectStackTraceResponse(t)
1169-
checkStackFrames(t, stack, "main.main", -1, 1000, 3, 3)
1254+
checkStackFramesExact(t, stack, "main.main", -1, 1000, 3, 3)
11701255

11711256
client.ScopesRequest(1000)
11721257
scopes := client.ExpectScopesResponse(t)
@@ -1439,7 +1524,7 @@ func TestScopesRequestsOptimized(t *testing.T) {
14391524
startLineno = -1
14401525
}
14411526

1442-
checkStackFrames(t, stack, "main.foobar", startLineno, 1000, 4, 4)
1527+
checkStackFramesExact(t, stack, "main.foobar", startLineno, 1000, 4, 4)
14431528

14441529
client.ScopesRequest(1000)
14451530
scopes := client.ExpectScopesResponse(t)
@@ -1454,7 +1539,7 @@ func TestScopesRequestsOptimized(t *testing.T) {
14541539
// Frame ids get reset at each breakpoint.
14551540
client.StackTraceRequest(1, 0, 20)
14561541
stack := client.ExpectStackTraceResponse(t)
1457-
checkStackFrames(t, stack, "main.barfoo", 27, 1000, 5, 5)
1542+
checkStackFramesExact(t, stack, "main.barfoo", 27, 1000, 5, 5)
14581543

14591544
client.ScopesRequest(1000)
14601545
scopes := client.ExpectScopesResponse(t)
@@ -1904,7 +1989,7 @@ func TestGlobalScopeAndVariables(t *testing.T) {
19041989
execute: func() {
19051990
client.StackTraceRequest(1, 0, 20)
19061991
stack := client.ExpectStackTraceResponse(t)
1907-
checkStackFrames(t, stack, "main.main", 36, 1000, 3, 3)
1992+
checkStackFramesExact(t, stack, "main.main", 36, 1000, 3, 3)
19081993

19091994
client.ScopesRequest(1000)
19101995
scopes := client.ExpectScopesResponse(t)
@@ -1926,7 +2011,7 @@ func TestGlobalScopeAndVariables(t *testing.T) {
19262011

19272012
client.StackTraceRequest(1, 0, 20)
19282013
stack = client.ExpectStackTraceResponse(t)
1929-
checkStackFrames(t, stack, "", 13, 1000, 4, 4)
2014+
checkStackFramesExact(t, stack, "", 13, 1000, 4, 4)
19302015

19312016
client.ScopesRequest(1000)
19322017
scopes = client.ExpectScopesResponse(t)
@@ -1970,7 +2055,7 @@ func TestShadowedVariables(t *testing.T) {
19702055
execute: func() {
19712056
client.StackTraceRequest(1, 0, 20)
19722057
stack := client.ExpectStackTraceResponse(t)
1973-
checkStackFrames(t, stack, "main.main", 13, 1000, 3, 3)
2058+
checkStackFramesExact(t, stack, "main.main", 13, 1000, 3, 3)
19742059

19752060
client.ScopesRequest(1000)
19762061
scopes := client.ExpectScopesResponse(t)
@@ -2010,7 +2095,7 @@ func TestLaunchRequestWithStackTraceDepth(t *testing.T) {
20102095
execute: func() {
20112096
client.StackTraceRequest(1, 0, 0)
20122097
stResp = client.ExpectStackTraceResponse(t)
2013-
checkStackFrames(t, stResp, "main.Increment", 8, 1000, 2 /*returned*/, 2 /*available*/)
2098+
checkStackFramesHasMore(t, stResp, "main.Increment", 8, 1000, 1 /*returned*/, 2 /*available*/)
20142099
},
20152100
disconnect: false,
20162101
}})

0 commit comments

Comments
 (0)