From c8f6c3a6853760d0ff8cd96368b8b17c86b599aa Mon Sep 17 00:00:00 2001 From: polinasok <51177946+polinasok@users.noreply.github.com> Date: Thu, 14 Oct 2021 10:58:53 -0700 Subject: [PATCH] service/dap: support remote-attaching to running debugger (#2737) Co-authored-by: Polina Sokolova --- service/dap/server.go | 37 ++++++----- service/dap/server_test.go | 132 +++++++++++++++++++++++++++++-------- 2 files changed, 125 insertions(+), 44 deletions(-) diff --git a/service/dap/server.go b/service/dap/server.go index ceed066c..8d9fc7b5 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -523,6 +523,18 @@ func (s *Session) handleRequest(request dap.Message) { // These requests, can be handled regardless of whether the targret is running switch request := request.(type) { + case *dap.InitializeRequest: + // Required + s.onInitializeRequest(request) + return + case *dap.LaunchRequest: + // Required + s.onLaunchRequest(request) + return + case *dap.AttachRequest: + // Required + s.onAttachRequest(request) + return case *dap.DisconnectRequest: // Required s.onDisconnectRequest(request) @@ -560,7 +572,7 @@ func (s *Session) handleRequest(request dap.Message) { // the next stop. In addition, the editor itself might block waiting // for these requests to return. We are not aware of any requests // that would benefit from this approach at this time. - if s.debugger != nil && s.isRunningCmd() { + if s.debugger != nil && s.debugger.IsRunning() || s.isRunningCmd() { switch request := request.(type) { case *dap.ThreadsRequest: // On start-up, the client requests the baseline of currently existing threads @@ -664,17 +676,6 @@ func (s *Session) handleRequest(request dap.Message) { }() <-resumeRequestLoop //--- Synchronous requests --- - // TODO(polina): target might be running when remote attach debug session - // is started. Support handling initialize and attach requests while running. - case *dap.InitializeRequest: - // Required - s.onInitializeRequest(request) - case *dap.LaunchRequest: - // Required - s.onLaunchRequest(request) - case *dap.AttachRequest: - // Required - s.onAttachRequest(request) case *dap.SetBreakpointsRequest: // Required s.onSetBreakpointsRequest(request) @@ -1514,7 +1515,7 @@ func closeIfOpen(ch chan struct{}) { // onConfigurationDoneRequest handles 'configurationDone' request. // This is an optional request enabled by capability ‘supportsConfigurationDoneRequest’. // It gets triggered after all the debug requests that follow initalized event, -// so the s.debugger is guaranteed to be set. +// so the s.debugger is guaranteed to be set. Expects the target to be halted. func (s *Session) onConfigurationDoneRequest(request *dap.ConfigurationDoneRequest, allowNextStateChange chan struct{}) { defer closeIfOpen(allowNextStateChange) if s.args.stopOnEntry { @@ -1705,10 +1706,14 @@ func (s *Session) onAttachRequest(request *dap.AttachRequest) { return } s.config.log.Debug("debugger already started") - // TODO(polina): once we allow initialize and attach request while running, - // halt before sending initialized event. onConfigurationDone will restart + // Halt for configuration sequence. onConfigurationDone will restart // execution if user requested !stopOnEntry. - + s.changeStateMu.Lock() + defer s.changeStateMu.Unlock() + if _, err := s.halt(); err != nil { + s.sendShowUserErrorResponse(request.Request, FailedToAttach, "Failed to attach", err.Error()) + return + } // Enable StepBack controls on supported backends if s.config.Debugger.Backend == "rr" { s.send(&dap.CapabilitiesEvent{Event: *newEvent("capabilities"), Body: dap.CapabilitiesEventBody{Capabilities: dap.Capabilities{SupportsStepBack: true}}}) diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 98925aaa..861eb4bc 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -4452,12 +4452,12 @@ func verifyStopLocation(t *testing.T, client *daptest.Client, thread int, name s // The details have been tested by other tests, // so this is just a sanity check. // Skips line check if line is -1. -func checkStop(t *testing.T, client *daptest.Client, thread int, name string, line int) { +func checkStop(t *testing.T, client *daptest.Client, thread int, fname string, line int) { t.Helper() client.ThreadsRequest() client.ExpectThreadsResponse(t) - verifyStopLocation(t, client, thread, name, line) + verifyStopLocation(t, client, thread, fname, line) client.ScopesRequest(1000) client.ExpectScopesResponse(t) @@ -5748,23 +5748,44 @@ func TestBadAttachRequest(t *testing.T) { }) } -// TODO(polina): also add launchDebuggerWithTargetRunning -func launchDebuggerWithTargetHalted(t *testing.T, fixture string) *debugger.Debugger { +func launchDebuggerWithTargetRunning(t *testing.T, fixture string) (*protest.Fixture, *debugger.Debugger) { t.Helper() - bin := protest.BuildFixture(fixture, protest.AllNonOptimized) + fixbin, dbg := launchDebuggerWithTargetHalted(t, fixture) + running := make(chan struct{}) + var err error + go func() { + t.Helper() + _, err = dbg.Command(&api.DebuggerCommand{Name: api.Continue}, running) + select { + case <-running: + default: + close(running) + } + }() + <-running + if err != nil { + t.Fatal("failed to continue on launch", err) + } + return fixbin, dbg +} + +func launchDebuggerWithTargetHalted(t *testing.T, fixture string) (*protest.Fixture, *debugger.Debugger) { + t.Helper() + fixbin := protest.BuildFixture(fixture, protest.AllNonOptimized) cfg := service.Config{ - ProcessArgs: []string{bin.Path}, + ProcessArgs: []string{fixbin.Path}, Debugger: debugger.Config{Backend: "default"}, } dbg, err := debugger.New(&cfg.Debugger, cfg.ProcessArgs) // debugger halts process on entry if err != nil { t.Fatal("failed to start debugger:", err) } - return dbg + return &fixbin, dbg } // runTestWithDebugger starts the server and sets its debugger, initializes a debug session, -// runs test, then disconnects. Expects the process at the end of test() to be halted. +// runs test, then disconnects. Expects no running async handler at the end of test() (either +// process is halted or debug session never launched.) func runTestWithDebugger(t *testing.T, dbg *debugger.Debugger, test func(c *daptest.Client)) { serverStopped := make(chan struct{}) server, _ := startDAPServer(t, serverStopped) @@ -5797,7 +5818,8 @@ func runTestWithDebugger(t *testing.T, dbg *debugger.Debugger, test func(c *dapt func TestAttachRemoteToHaltedTargetStopOnEntry(t *testing.T) { // Halted + stop on entry - runTestWithDebugger(t, launchDebuggerWithTargetHalted(t, "increment"), func(client *daptest.Client) { + _, dbg := launchDebuggerWithTargetHalted(t, "increment") + runTestWithDebugger(t, dbg, func(client *daptest.Client) { client.AttachRequest(map[string]interface{}{"mode": "remote", "stopOnEntry": true}) client.ExpectInitializedEvent(t) client.ExpectAttachResponse(t) @@ -5809,7 +5831,8 @@ func TestAttachRemoteToHaltedTargetStopOnEntry(t *testing.T) { func TestAttachRemoteToHaltedTargetContinueOnEntry(t *testing.T) { // Halted + continue on entry - runTestWithDebugger(t, launchDebuggerWithTargetHalted(t, "http_server"), func(client *daptest.Client) { + _, dbg := launchDebuggerWithTargetHalted(t, "http_server") + runTestWithDebugger(t, dbg, func(client *daptest.Client) { client.AttachRequest(map[string]interface{}{"mode": "remote", "stopOnEntry": false}) client.ExpectInitializedEvent(t) client.ExpectAttachResponse(t) @@ -5823,7 +5846,41 @@ func TestAttachRemoteToHaltedTargetContinueOnEntry(t *testing.T) { }) } -// TODO(polina): Running + stop/continue on entry +func TestAttachRemoteToRunningTargetStopOnEntry(t *testing.T) { + fixture, dbg := launchDebuggerWithTargetRunning(t, "loopprog") + runTestWithDebugger(t, dbg, func(client *daptest.Client) { + client.AttachRequest(map[string]interface{}{"mode": "remote", "stopOnEntry": true}) + client.ExpectInitializedEvent(t) + client.ExpectAttachResponse(t) + // Target is halted here + client.SetBreakpointsRequest(fixture.Source, []int{8}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{8, fixture.Source, true, ""}}) + client.ConfigurationDoneRequest() + client.ExpectStoppedEvent(t) + client.ExpectConfigurationDoneResponse(t) + client.ContinueRequest(1) + client.ExpectContinueResponse(t) + client.ExpectStoppedEvent(t) + checkStop(t, client, 1, "main.loop", 8) + }) +} + +func TestAttachRemoteToRunningTargetContinueOnEntry(t *testing.T) { + fixture, dbg := launchDebuggerWithTargetRunning(t, "loopprog") + runTestWithDebugger(t, dbg, func(client *daptest.Client) { + client.AttachRequest(map[string]interface{}{"mode": "remote", "stopOnEntry": false}) + client.ExpectInitializedEvent(t) + client.ExpectAttachResponse(t) + // Target is halted here + client.SetBreakpointsRequest(fixture.Source, []int{8}) + expectSetBreakpointsResponse(t, client, []Breakpoint{{8, fixture.Source, true, ""}}) + client.ConfigurationDoneRequest() + // Target is restarted here + client.ExpectConfigurationDoneResponse(t) + client.ExpectStoppedEvent(t) + checkStop(t, client, 1, "main.loop", 8) + }) +} // TestMultiClient tests that that remote attach doesn't take down // the server in multi-client mode unless terminateDebugee is explicitely set. @@ -5853,7 +5910,7 @@ func TestAttachRemoteMultiClient(t *testing.T) { // hack to test the inner connection logic that can be used by a server that does. server.session.config.AcceptMulti = true // TODO(polina): update once the server interface is refactored to take debugger as arg - server.session.debugger = launchDebuggerWithTargetHalted(t, "increment") + _, server.session.debugger = launchDebuggerWithTargetHalted(t, "increment") server.sessionMu.Unlock() client.InitializeRequest() @@ -5887,25 +5944,44 @@ func TestAttachRemoteMultiClient(t *testing.T) { } func TestLaunchAttachErrorWhenDebugInProgress(t *testing.T) { - runTestWithDebugger(t, launchDebuggerWithTargetHalted(t, "increment"), func(client *daptest.Client) { - client.AttachRequest(map[string]interface{}{"mode": "local", "processId": 100}) - er := client.ExpectVisibleErrorResponse(t) - msg := "Failed to attach: debugger already started - use remote mode to connect" - if er.Body.Error.Id != FailedToAttach || er.Body.Error.Format != msg { - t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToAttach, msg) - } - tests := []string{"debug", "test", "exec", "replay", "core"} - for _, mode := range tests { - t.Run(mode, func(t *testing.T) { - client.LaunchRequestWithArgs(map[string]interface{}{"mode": mode}) + tests := []struct { + name string + dbg func() *debugger.Debugger + }{ + {"halted", func() *debugger.Debugger { _, dbg := launchDebuggerWithTargetHalted(t, "increment"); return dbg }}, + {"running", func() *debugger.Debugger { _, dbg := launchDebuggerWithTargetRunning(t, "loopprog"); return dbg }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + runTestWithDebugger(t, tc.dbg(), func(client *daptest.Client) { + client.EvaluateRequest("1==1", 0 /*no frame specified*/, "repl") + if tc.name == "running" { + client.ExpectInvisibleErrorResponse(t) + } else { + client.ExpectEvaluateResponse(t) + } + + // Both launch and attach requests should go through for additional error checking + client.AttachRequest(map[string]interface{}{"mode": "local", "processId": 100}) er := client.ExpectVisibleErrorResponse(t) - msg := "Failed to launch: debugger already started - use remote attach to connect to a server with an active debug session" - if er.Body.Error.Id != FailedToLaunch || er.Body.Error.Format != msg { - t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToLaunch, msg) + msg := "Failed to attach: debugger already started - use remote mode to connect" + if er.Body.Error.Id != FailedToAttach || er.Body.Error.Format != msg { + t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToAttach, msg) + } + tests := []string{"debug", "test", "exec", "replay", "core"} + for _, mode := range tests { + t.Run(mode, func(t *testing.T) { + client.LaunchRequestWithArgs(map[string]interface{}{"mode": mode}) + er := client.ExpectVisibleErrorResponse(t) + msg := "Failed to launch: debugger already started - use remote attach to connect to a server with an active debug session" + if er.Body.Error.Id != FailedToLaunch || er.Body.Error.Format != msg { + t.Errorf("got %#v, want Id=%d Format=%q", er, FailedToLaunch, msg) + } + }) } }) - } - }) + }) + } } func TestBadInitializeRequest(t *testing.T) {