Skip to content

Commit

Permalink
dap: change how noDebug launch request is served
Browse files Browse the repository at this point in the history 
PR go-delve#2400 added support for noDebug launch requests - that was done
by directly starting the target program using os/exec.Cmd and blocking
the launch request indefinitely until the program terminates or
is stopped with a disconnect request (when dlv dap can support it).
Even though the approach seemed to work for user, that adds an extra
control flow and complexity to the codebase.

This change takes a different approach to implement the noDebug
launch feature. Instead of using os/exec.Cmd, this uses the existing
debug launch path, but avoids setting breakpoints. Finally, the program
will start upon receiving onConfigurationDoneRequest and blocks there
until the program terminates. This simplifies the code.

The DAP spec does not explicitly specify what to do about other
requests. It seems like VSCode can issue evaluate requests or other
requests after the configuration done request. Currently they are
blocked because the program is in running state. We can consider checking
s.noDebug and responding with an error in the future if we want/need to.

See the log below for a typical DAP request/response sequence:

2021-03-29T01:42:53-04:00 debug layer=dap building binary at /Users/hakim/projects/s/__debug_bin
2021-03-29T01:42:55-04:00 info layer=debugger launching process with args: [/Users/hakim/projects/s/__debug_bin]
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"event","event":"initialized"}
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":2,"success":true,"command":"launch"}
2021-03-29T01:42:55-04:00 debug layer=dap [<- from client]{"seq":3,"type":"request","command":"setBreakpoints","arguments":{"source":{"name":"main.go","path":"/Users/hakim/projects/s/main.go"},"breakpoints":[{"line":9}],"lines":[9]}}
2021-03-29T01:42:55-04:00 error layer=dap Unable to set or clear breakpoints: running in noDebug mode
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":3,"success":false,"command":"setBreakpoints","message":"Unable to set or clear breakpoints","body":{"error":{"id":2002,"format":"Unable to set or clear breakpoints: running in noDebug mode"}}}
2021-03-29T01:42:55-04:00 debug layer=dap [<- from client]{"seq":4,"type":"request","command":"configurationDone","arguments":{}}
2021-03-29T01:42:55-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":4,"success":true,"command":"configurationDone"}
2021-03-29T01:42:55-04:00 debug layer=debugger continuing
Hello
2021-03-29T01:43:00-04:00 debug layer=dap [-> to client]{"seq":0,"type":"event","event":"terminated","body":{}}
2021-03-29T01:43:00-04:00 debug layer=dap [<- from client]{"seq":5,"type":"request","command":"threads"}
2021-03-29T01:43:00-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":5,"success":true,"command":"threads","body":{"threads":null}}
2021-03-29T01:43:00-04:00 debug layer=dap [<- from client]{"seq":6,"type":"request","command":"disconnect","arguments":{}}
2021-03-29T01:43:00-04:00 debug layer=dap [-> to client]{"seq":0,"type":"response","request_seq":6,"success":true,"command":"disconnect"}
2021-03-29T01:43:00-04:00 debug layer=debugger halting
2021-03-29T01:43:00-04:00 error layer=dap Process 27219 has exited with status 0
2021-03-29T01:43:00-04:00 debug layer=debugger detaching

Updates golang/vscode-go#1111
  • Loading branch information
@hyangah
hyangah committed Apr 5, 2021
1 parent c223ef6 commit 08348b8
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 78 deletions.
101 changes: 48 additions & 53 deletions service/dap/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"regexp"
"runtime"
"strings"
"sync"

"github.com/go-delve/delve/pkg/gobuild"
"github.com/go-delve/delve/pkg/logflags"
Expand Down Expand Up @@ -71,10 +70,9 @@ type Server struct {
// args tracks special settings for handling debug session requests.
args launchAttachArgs

mu sync.Mutex

// noDebugProcess is set for the noDebug launch process.
noDebugProcess *exec.Cmd
noDebugProcess *exec.Cmd
noDebugProcessTerminated bool
}

// launchAttachArgs captures arguments from launch/attach request that
Expand Down Expand Up @@ -167,6 +165,9 @@ func (s *Server) Stop() {
}
}
}
if s.noDebugProcess != nil {
s.stopNoDebugProcess()
}
}

// signalDisconnect closes config.DisconnectChan if not nil, which
Expand Down Expand Up @@ -554,75 +555,58 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
s.config.Debugger.WorkingDir = wdParsed
}

noDebug, ok := request.Arguments["noDebug"]
if ok {
if v, ok := noDebug.(bool); ok && v { // noDebug == true
if err := s.runWithoutDebug(program, targetArgs, s.config.Debugger.WorkingDir); err != nil {
s.sendErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error())
} else { // program terminated.
s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")})
}
return
noDebug := false
if noDebugArg, ok := request.Arguments["noDebug"]; ok {
if v, ok := noDebugArg.(bool); ok && v { // noDebug == true
noDebug = true
}
}

var err error
if s.debugger, err = debugger.New(&s.config.Debugger, s.config.ProcessArgs); err != nil {
s.sendErrorResponse(request.Request,
FailedToLaunch, "Failed to launch", err.Error())
return
if noDebug {
if s.noDebugProcess, err = s.launchWithoutDebug(program, targetArgs, s.config.Debugger.WorkingDir); err != nil {
s.sendErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error())
return
}
} else {
if s.debugger, err = debugger.New(&s.config.Debugger, s.config.ProcessArgs); err != nil {
s.sendErrorResponse(request.Request, FailedToLaunch, "Failed to launch", err.Error())
return
}
}

// Notify the client that the debugger is ready to start accepting
// configuration requests for setting breakpoints, etc. The client
// will end the configuration sequence with 'configurationDone'.
s.send(&dap.InitializedEvent{Event: *newEvent("initialized")})
s.send(&dap.LaunchResponse{Response: *newResponse(request.Request)})
// TODO: if the client doesn't have supportConfigurationDone capability,
// start the debugging here.
}

func (s *Server) runWithoutDebug(program string, targetArgs []string, wd string) error {
func (s *Server) launchWithoutDebug(program string, targetArgs []string, wd string) (*exec.Cmd, error) {
s.log.Debugln("Running without debug: ", program)

cmd := exec.Command(program, targetArgs...)
// TODO: send stdin/out/err as OutputEvent messages
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.Stdin = os.Stdin
cmd.Dir = s.config.Debugger.WorkingDir

s.mu.Lock()
defer s.mu.Unlock()
if s.noDebugProcess != nil {
return fmt.Errorf("previous process (pid=%v) is still active", s.noDebugProcess.Process.Pid)
}
if err := cmd.Start(); err != nil {
return err
}
s.noDebugProcess = cmd
s.mu.Unlock() // allow disconnect or restart requests to call stopNoDebugProcess.

// block until the process terminates.
if err := cmd.Wait(); err != nil {
s.log.Debugf("process exited with %v", err)
}
cmd.Dir = wd
return cmd, nil
}

s.mu.Lock()
if s.noDebugProcess == cmd {
s.noDebugProcess = nil
}
return nil // Program ran and terminated.
func (s *Server) runNoDebugProcess() error {
err := s.noDebugProcess.Run()
s.noDebugProcessTerminated = true
return err
}

func (s *Server) stopNoDebugProcess() {
s.mu.Lock()
defer s.mu.Unlock()
if s.noDebugProcess == nil {
if s.noDebugProcess == nil || s.noDebugProcessTerminated {
return
}

// TODO(hyangah): gracefully terminate the process and its children processes.
if err := s.noDebugProcess.Process.Kill(); err != nil {
s.log.Debugf("killing process (pid=%v) failed: %v", s.noDebugProcess.Process.Pid, err)
}
s.noDebugProcessTerminated = true
}

// TODO(polina): support "remote" mode
Expand Down Expand Up @@ -667,14 +651,16 @@ func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) {
s.log.Error(err)
}
}
} else {
s.stopNoDebugProcess()
}
// TODO(polina): make thread-safe when handlers become asynchronous.
s.signalDisconnect()
}

func (s *Server) onSetBreakpointsRequest(request *dap.SetBreakpointsRequest) {
if s.noDebugProcess != nil {
s.sendErrorResponse(request.Request, UnableToSetBreakpoints, "Unable to set or clear breakpoints", "running in noDebug mode")
return
}
// TODO(polina): handle this while running by halting first.

if request.Arguments.Source.Path == "" {
Expand Down Expand Up @@ -738,16 +724,21 @@ func (s *Server) onSetExceptionBreakpointsRequest(request *dap.SetExceptionBreak
}

func (s *Server) onConfigurationDoneRequest(request *dap.ConfigurationDoneRequest) {
if s.args.stopOnEntry {
if s.debugger != nil && s.args.stopOnEntry {
e := &dap.StoppedEvent{
Event: *newEvent("stopped"),
Body: dap.StoppedEventBody{Reason: "entry", ThreadId: 1, AllThreadsStopped: true},
}
s.send(e)
}
s.send(&dap.ConfigurationDoneResponse{Response: *newResponse(request.Request)})
if !s.args.stopOnEntry {
if s.debugger != nil && !s.args.stopOnEntry {
s.doCommand(api.Continue)
} else if s.noDebugProcess != nil {
if err := s.runNoDebugProcess(); err != nil { // block until program terminates.
s.log.Errorf("failed to start target: %v", err)
}
s.send(&dap.TerminatedEvent{Event: *newEvent("terminated")})
}
}

Expand All @@ -767,7 +758,11 @@ func fnName(loc *proc.Location) string {

func (s *Server) onThreadsRequest(request *dap.ThreadsRequest) {
if s.debugger == nil {
s.sendErrorResponse(request.Request, UnableToDisplayThreads, "Unable to display threads", "debugger is nil")
msg := "debugger is nil"
if s.noDebugProcess != nil {
msg = "running in noDebug mode"
}
s.sendErrorResponse(request.Request, UnableToDisplayThreads, "Unable to display threads", msg)
return
}
gs, _, err := s.debugger.Goroutines(0, 0)
Expand Down
43 changes: 18 additions & 25 deletions service/dap/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2553,44 +2553,37 @@ func TestLaunchRequestDefaults(t *testing.T) {

func TestLaunchRequestDefaultsNoDebug(t *testing.T) {
runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
runNoDebugDebugSession(t, client, "launch", func() {
runNoDebugDebugSession(t, client, func() {
client.LaunchRequestWithArgs(map[string]interface{}{
"noDebug": true,
"mode": "", /*"debug" by default*/
"program": fixture.Source,
"output": cleanExeName("__mybin")})
}, fixture.Source)
})
runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
runNoDebugDebugSession(t, client, "launch", func() {
// Use the default output directory.
client.LaunchRequestWithArgs(map[string]interface{}{
"noDebug": true,
/*"mode":"debug" by default*/
"program": fixture.Source,
"output": cleanExeName("__mybin")})
}, fixture.Source)
})
runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
runNoDebugDebugSession(t, client, "launch", func() {
// Use the default output directory.
client.LaunchRequestWithArgs(map[string]interface{}{
"noDebug": true,
"mode": "debug",
"program": fixture.Source})
// writes to default output dir __debug_bin
}, fixture.Source)
}, fixture.Source, []int{8})
})
}

func runNoDebugDebugSession(t *testing.T, client *daptest.Client, cmd string, cmdRequest func(), source string) {
// 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) {
client.InitializeRequest()
client.ExpectInitializeResponse(t)

cmdRequest()
// ! client.InitializedEvent.
// ! client.ExpectLaunchResponse
client.ExpectInitializedEvent(t)
// noDebug mode applies only to "launch" requests.
client.ExpectLaunchResponse(t)

// Breakpoints shouldn't be set.
client.SetBreakpointsRequest(source, breakpoints)
client.ExpectErrorResponse(t)

client.ConfigurationDoneRequest()
client.ExpectConfigurationDoneResponse(t)

client.ExpectTerminatedEvent(t)
client.DisconnectRequestWithKillOption(true)
client.ExpectDisconnectResponse(t)
}

func TestLaunchTestRequest(t *testing.T) {
Expand Down

0 comments on commit 08348b8

Please sign in to comment.