Repository: go-delve/delve
Pull request #2400 (2021-03-22 16:35:51)

service/dap: supports noDebug launch requests
Following the DAP spec 

https://microsoft.github.io/debug-adapter-protocol/specification

Launch Request
This launch request is sent from the client to the debug adapter to 
start the debuggee with or without debugging (if ‘noDebug’ is true).

This is admittedly strange to be implemented inside delve (debugger),
but implementing this outside `dlv dap`  (from the editor plugin side)
implies the need for another DAP layer, which isn't desirable either.

If the launch requests has noDebug attribute set, run the built
binary directly. The launch request handler will block until
the binary terminates, so the editor won't send additional requests
like breakpoint setting etc. Still disconnect or restart requests
can flow in though and they should trigger killing of the target
process if it's still running.

In order to run the binary using os/exec on windows, the target
binary has to have .exe as its extension. So, add .exe to the default
output name if it is on windows. I am not sure though yet we want
to modify the user-specified output or not yet. Considering how
go commands behave (not automatically append .exe for 'go build -o')
I think respecting what user specified is right, but the failure
(file not exist) may be mysterious.

hyangah: Finally, all checks have passed. 
This brings in more changes than I hoped due to the windows issue. (i.e. os/exec.Command.Start can't locate the built binary unless it's .exe file. Alternative is to let the debugger Launch & continue, but not return until the debugee terminates. That will prevent the editor from sending breakpoints and other requests.)

@polinasok @suzmue 
derekparker: Can you bring some context here as to the motivation behind this change? I'm unsure of why we would want Delve to handle simple process creation without debugging that process at all.
hyangah: @derekparker This is a strange side of DAP spec. :-(

https://microsoft.github.io/debug-adapter-protocol/specification
* Launch Request
This launch request is sent from the client to the debug adapter to start the debuggee with or without debugging (if ‘noDebug’ is true).
...
```
interface LaunchRequestArguments {
  /**
   * If noDebug is true the launch request should launch the program without
   * enabling debugging.
   */
  noDebug?: boolean;

  /**
   * Optional data from the previous, restarted session.
   * The data is sent as the 'restart' attribute of the 'terminated' event.
   * The client should leave the data intact.
   */
  __restart?: any;
}
Response to ‘launch’ request. This is just an acknowledgement, so no body field is required.

```
derekparker: @hyangah ack, thanks for the explanation! Can you please rebase this PR to solve the conflicts.
hyangah: @derekparker Thanks. Rebased. 

Patch:
diff --git a/service/dap/server.go b/service/dap/server.go
index ba8d3451..69bf59e3 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -16,10 +16,13 @@ import (
 	"io"
 	"net"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"reflect"
 	"regexp"
+	"runtime"
 	"strings"
+	"sync"
 
 	"github.com/go-delve/delve/pkg/gobuild"
 	"github.com/go-delve/delve/pkg/logflags"
@@ -67,6 +70,11 @@ type Server struct {
 	variableHandles *variablesHandlesMap
 	// 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
 }
 
 // launchAttachArgs captures arguments from launch/attach request that
@@ -431,6 +439,13 @@ func (s *Server) onInitializeRequest(request *dap.InitializeRequest) {
 // Output path for the compiled binary in debug or test modes.
 const debugBinary string = "./__debug_bin"
 
+func cleanExeName(name string) string {
+	if runtime.GOOS == "windows" && filepath.Ext(name) != ".exe" {
+		return name + ".exe"
+	}
+	return name
+}
+
 func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 	// Validate launch request mode
 	mode, ok := request.Arguments["mode"]
@@ -456,7 +471,7 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 	if mode == "debug" || mode == "test" {
 		output, ok := request.Arguments["output"].(string)
 		if !ok || output == "" {
-			output = debugBinary
+			output = cleanExeName(debugBinary)
 		}
 		debugname, err := filepath.Abs(output)
 		if err != nil {
@@ -532,6 +547,17 @@ 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
+		}
+	}
 	var err error
 	if s.debugger, err = debugger.New(&s.config.Debugger, s.config.ProcessArgs); err != nil {
 		s.sendErrorResponse(request.Request,
@@ -546,6 +572,52 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 	s.send(&dap.LaunchResponse{Response: *newResponse(request.Request)})
 }
 
+func (s *Server) runWithoutDebug(program string, targetArgs []string, wd string) error {
+	s.log.Println("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.Errorf("process exited with %v", err)
+	}
+
+	s.mu.Lock()
+	if s.noDebugProcess == cmd {
+		s.noDebugProcess = nil
+	}
+	return nil // Program ran and terminated.
+}
+
+func (s *Server) stopNoDebugProcess() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.noDebugProcess == nil {
+		return
+	}
+
+	// TODO(hyangah): gracefully terminate the process and its children processes.
+	if err := s.noDebugProcess.Process.Kill(); err != nil {
+		s.log.Errorf("killing process (pid=%v) failed: %v", s.noDebugProcess.Process.Pid, err)
+	}
+}
+
 // TODO(polina): support "remote" mode
 func isValidLaunchMode(launchMode interface{}) bool {
 	switch launchMode {
@@ -578,6 +650,8 @@ func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) {
 		if err != nil {
 			s.log.Error(err)
 		}
+	} else {
+		s.stopNoDebugProcess()
 	}
 	// TODO(polina): make thread-safe when handlers become asynchronous.
 	s.signalDisconnect()


Test patch:
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index 9290b305..6f1d640d 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -1693,7 +1693,7 @@ func TestWorkingDir(t *testing.T) {
 					"mode":        "exec",
 					"program":     fixture.Path,
 					"stopOnEntry": false,
-					"wd":         wd,
+					"wd":          wd,
 				})
 			},
 			// Set breakpoints
@@ -2366,6 +2366,48 @@ 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() {
+			client.LaunchRequestWithArgs(map[string]interface{}{
+				"noDebug": true,
+				"mode":    "", /*"debug" by default*/
+				"program": fixture.Source,
+				"output":  cleanExeName("__mydir")})
+		}, 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("__mydir")})
+		}, 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)
+	})
+}
+
+func runNoDebugDebugSession(t *testing.T, client *daptest.Client, cmd string, cmdRequest func(), source string) {
+	client.InitializeRequest()
+	client.ExpectInitializeResponse(t)
+
+	cmdRequest()
+	// ! client.InitializedEvent.
+	// ! client.ExpectLaunchResponse
+	client.ExpectTerminatedEvent(t)
+}
+
 func TestLaunchTestRequest(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
 		runDebugSession(t, client, "launch", func() {