diff --git a/service/dap/daptest/client.go b/service/dap/daptest/client.go index a99bf4cd2f..e2390c8ffd 100644 --- a/service/dap/daptest/client.go +++ b/service/dap/daptest/client.go @@ -8,6 +8,7 @@ import ( "log" "net" "path/filepath" + "regexp" "testing" "github.com/google/go-dap" @@ -170,6 +171,35 @@ func (c *Client) ExpectOutputEvent(t *testing.T) *dap.OutputEvent { return c.ExpectMessage(t).(*dap.OutputEvent) } +func (c *Client) ExpectOutputEventRegex(t *testing.T, want string) *dap.OutputEvent { + t.Helper() + e := c.ExpectMessage(t).(*dap.OutputEvent) + if matched, _ := regexp.MatchString(want, e.Body.Output); !matched { + t.Errorf("\ngot %#v\nwant Output=%q", e, want) + } + return e +} + +func (c *Client) ExpectOutputEventProcessExited(t *testing.T, status int) *dap.OutputEvent { + t.Helper() + return c.ExpectOutputEventRegex(t, fmt.Sprintf(`Process [0-9]+ has exited with status %d\n`, status)) +} + +func (c *Client) ExpectOutputEventDetaching(t *testing.T) *dap.OutputEvent { + t.Helper() + return c.ExpectOutputEventRegex(t, `Detaching\n`) +} + +func (c *Client) ExpectOutputEventDetachingKill(t *testing.T) *dap.OutputEvent { + t.Helper() + return c.ExpectOutputEventRegex(t, `Detaching and terminating target process\n`) +} + +func (c *Client) ExpectOutputEventDetachingNoKill(t *testing.T) *dap.OutputEvent { + t.Helper() + return c.ExpectOutputEventRegex(t, `Detaching without terminating target process\n`) +} + func (c *Client) ExpectConfigurationDoneResponse(t *testing.T) *dap.ConfigurationDoneResponse { t.Helper() return c.ExpectMessage(t).(*dap.ConfigurationDoneResponse) diff --git a/service/dap/error_ids.go b/service/dap/error_ids.go index 8b1ac9282d..03ec41e719 100644 --- a/service/dap/error_ids.go +++ b/service/dap/error_ids.go @@ -21,4 +21,5 @@ const ( UnableToLookupVariable = 2008 UnableToEvaluateExpression = 2009 // Add more codes as we support more requests + DisconnectError = 5000 ) diff --git a/service/dap/server.go b/service/dap/server.go index 664944d4e2..ef3932ac14 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -147,6 +147,7 @@ func (s *Server) setLaunchAttachArgs(request dap.LaunchAttachRequest) { // process if it was launched by it. This method mustn't be called more than // once. func (s *Server) Stop() { + s.log.Debug("DAP server stopping...") _ = s.listener.Close() close(s.stopChan) if s.conn != nil { @@ -169,6 +170,7 @@ func (s *Server) Stop() { } else { s.stopNoDebugProcess() } + s.log.Debug("DAP server stopped") } // signalDisconnect closes config.DisconnectChan if not nil, which @@ -421,6 +423,15 @@ func (s *Server) send(message dap.Message) { _ = dap.WriteProtocolMessage(s.conn, message) } +func (s *Server) logToConsole(msg string) { + s.send(&dap.OutputEvent{ + Event: *newEvent("output"), + Body: dap.OutputEventBody{ + Output: msg + "\n", + Category: "console", + }}) +} + func (s *Server) onInitializeRequest(request *dap.InitializeRequest) { // TODO(polina): Respond with an error if debug session is in progress? response := &dap.InitializeResponse{Response: *newResponse(request.Request)} @@ -568,9 +579,8 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) { // Then, block until the program terminates or is stopped. if err := cmd.Wait(); err != nil { - s.log.Debugf("program exited: %v", err) + s.log.Debugf("program exited with error: %v", err) } - stopped := false s.mu.Lock() stopped = s.noDebugProcess == nil // if it was stopped, this should be nil. @@ -578,6 +588,7 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) { s.mu.Unlock() if !stopped { + s.logToConsole(proc.ErrProcessExited{Pid: cmd.ProcessState.Pid(), Status: cmd.ProcessState.ExitCode()}.Error()) s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")}) } return @@ -613,8 +624,16 @@ func (s *Server) stopNoDebugProcess() { s.noDebugProcess = nil defer s.mu.Unlock() - // TODO(hyangah): gracefully terminate the process and its children processes. - if p != nil && !p.ProcessState.Exited() { + if p == nil { + // We already handled termination or there was never a process + return + } + + if p.ProcessState.Exited() { + s.logToConsole(proc.ErrProcessExited{Pid: p.ProcessState.Pid(), Status: p.ProcessState.ExitCode()}.Error()) + } else { + // TODO(hyangah): gracefully terminate the process and its children processes. + s.logToConsole(fmt.Sprintf("Terminating process %d", p.Process.Pid)) p.Process.Kill() // Don't check error. Process killing and self-termination may race. } } @@ -633,30 +652,42 @@ func isValidLaunchMode(launchMode interface{}) bool { // it disconnects the debuggee and signals that the debug adaptor // (in our case this TCP server) can be terminated. func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) { - s.send(&dap.DisconnectResponse{Response: *newResponse(request.Request)}) + var err error + var exited error if s.debugger != nil { - _, err := s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil) + state, err := s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil) if err != nil { switch err.(type) { case *proc.ErrProcessExited: - s.log.Debug(err) + exited = err default: s.log.Error(err) } + } else if state.Exited { + exited = proc.ErrProcessExited{Pid: s.debugger.ProcessPid(), Status: state.ExitStatus} } // We always kill launched programs kill := s.config.Debugger.AttachPid == 0 // In case of attach, we leave the program - // running but default, which can be + // running by default, which can be // overridden by an explicit request to terminate. if request.Arguments.TerminateDebuggee { kill = true } + if exited != nil { + s.logToConsole(exited.Error()) + s.logToConsole("Detaching") + } else if kill { + s.logToConsole("Detaching and terminating target process") + } else { + s.logToConsole("Detaching without terminating target processs") + } err = s.debugger.Detach(kill) if err != nil { switch err.(type) { case *proc.ErrProcessExited: - s.log.Debug(err) + exited = err + s.logToConsole(exited.Error()) default: s.log.Error(err) } @@ -664,6 +695,11 @@ func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) { } else { s.stopNoDebugProcess() } + if err != nil && exited == nil { + s.sendErrorResponse(request.Request, DisconnectError, "Error while disconnecting", err.Error()) + } else { + s.send(&dap.DisconnectResponse{Response: *newResponse(request.Request)}) + } // TODO(polina): make thread-safe when handlers become asynchronous. s.signalDisconnect() diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 7c5e32653f..6a74ce3634 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -113,6 +113,8 @@ func runTest(t *testing.T, name string, test func(c *daptest.Client, f protest.F // : 12 << continue // - Program runs to completion : << terminated event // : 13 >> disconnect +// : << output event (Process exited) +// : << output event (Detaching) // : 13 << disconnect // This test exhaustively tests Seq and RequestSeq on all messages from the // server. Other tests do not necessarily need to repeat all these checks. @@ -222,6 +224,14 @@ func TestLaunchStopOnEntry(t *testing.T) { // 13 >> disconnect, << disconnect client.DisconnectRequest() + oep := client.ExpectOutputEventProcessExited(t, 0) + if oep.Seq != 0 || oep.Body.Category != "console" { + t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oep) + } + oed := client.ExpectOutputEventDetaching(t) + if oed.Seq != 0 || oep.Body.Category != "console" { + t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oed) + } dResp := client.ExpectDisconnectResponse(t) if dResp.Seq != 0 || dResp.RequestSeq != 13 { t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=13", dResp) @@ -344,6 +354,10 @@ func TestAttachStopOnEntry(t *testing.T) { // 12 >> disconnect, << disconnect client.DisconnectRequestWithKillOption(true) + oed := client.ExpectOutputEventDetachingKill(t) + if oed.Seq != 0 || oed.Body.Category != "console" { + t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oed) + } dResp := client.ExpectDisconnectResponse(t) if dResp.Seq != 0 || dResp.RequestSeq != 12 { t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=12", dResp) @@ -398,6 +412,8 @@ func TestContinueOnEntry(t *testing.T) { // 7 >> disconnect, << disconnect client.DisconnectRequest() + client.ExpectOutputEventProcessExited(t, 0) + client.ExpectOutputEventDetaching(t) dResp := client.ExpectDisconnectResponse(t) if dResp.Seq != 0 || dResp.RequestSeq != 7 { t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=7", dResp) @@ -511,6 +527,8 @@ func TestPreSetBreakpoint(t *testing.T) { client.ExpectTerminatedEvent(t) client.DisconnectRequest() + client.ExpectOutputEventProcessExited(t, 0) + client.ExpectOutputEventDetaching(t) client.ExpectDisconnectResponse(t) }) } @@ -2019,8 +2037,8 @@ func TestEvaluateCallRequest(t *testing.T) { } // Call can exit client.EvaluateRequest("call callexit()", 1000, "this context will be ignored") - client.ExpectTerminatedEvent(t) }, + terminated: true, disconnect: true, }}) }) @@ -2428,9 +2446,12 @@ func handleStop(t *testing.T, client *daptest.Client, thread int, name string, l // disconnect is true, the test harness will abort the program // execution. Otherwise, a continue will be issued and the // program will continue to the next breakpoint or termination. +// If terminated is true, we expect requests at this breakpoint +// to result in termination. type onBreakpoint struct { execute func() disconnect bool + terminated bool } // runDebugSessionWithBPs is a helper for executing the common init and shutdown @@ -2475,7 +2496,16 @@ func runDebugSessionWithBPs(t *testing.T, client *daptest.Client, cmd string, cm client.ExpectStoppedEvent(t) onBP.execute() if onBP.disconnect { + if onBP.terminated { + client.ExpectTerminatedEvent(t) + } client.DisconnectRequestWithKillOption(true) + if onBP.terminated { + client.ExpectOutputEventProcessExited(t, 0) + client.ExpectOutputEventDetaching(t) + } else { + client.ExpectOutputEventDetachingKill(t) + } client.ExpectDisconnectResponse(t) return } @@ -2488,6 +2518,12 @@ func runDebugSessionWithBPs(t *testing.T, client *daptest.Client, cmd string, cm client.ExpectTerminatedEvent(t) } client.DisconnectRequestWithKillOption(true) + if cmd == "launch" { + client.ExpectOutputEventProcessExited(t, 0) + client.ExpectOutputEventDetaching(t) + } else if cmd == "attach" { + client.ExpectOutputEventDetachingKill(t) + } client.ExpectDisconnectResponse(t) } @@ -2567,21 +2603,33 @@ func TestLaunchRequestDefaults(t *testing.T) { }) } -func TestLaunchRequestNoDebug(t *testing.T) { +func TestLaunchRequestNoDebug_GoodStatus(t *testing.T) { runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) { runNoDebugDebugSession(t, client, func() { client.LaunchRequestWithArgs(map[string]interface{}{ "noDebug": true, - "mode": "", /*"debug" by default*/ + "mode": "debug", + "program": fixture.Source, + "output": cleanExeName("__mybin")}) + }, fixture.Source, []int{8}, 0) + }) +} + +func TestLaunchRequestNoDebug_BadStatus(t *testing.T) { + runTest(t, "issue1101", func(client *daptest.Client, fixture protest.Fixture) { + runNoDebugDebugSession(t, client, func() { + client.LaunchRequestWithArgs(map[string]interface{}{ + "noDebug": true, + "mode": "debug", "program": fixture.Source, "output": cleanExeName("__mybin")}) - }, fixture.Source, []int{8}) + }, fixture.Source, []int{8}, 2) }) } // runNoDebugDebugSession tests the session started with noDebug=true runs uninterrupted // even when breakpoint is set. -func runNoDebugDebugSession(t *testing.T, client *daptest.Client, cmdRequest func(), source string, breakpoints []int) { +func runNoDebugDebugSession(t *testing.T, client *daptest.Client, cmdRequest func(), source string, breakpoints []int, status int) { client.InitializeRequest() client.ExpectInitializeResponse(t) @@ -2590,6 +2638,7 @@ func runNoDebugDebugSession(t *testing.T, client *daptest.Client, cmdRequest fun // noDebug mode applies only to "launch" requests. client.ExpectLaunchResponse(t) + client.ExpectOutputEventProcessExited(t, status) client.ExpectTerminatedEvent(t) client.DisconnectRequestWithKillOption(true) client.ExpectDisconnectResponse(t)