[WIP] pkg/proc: avoid target process leaks. (#2018)

* pkg/proc: avoid target process leaks.

Target process should exit when dlv launch failed.

Fix #2017.

_fixtures/http_server.go

@@ -0,0 +1,13 @@
+package main
+
+import (
+	"fmt"
+	"net/http"
+)
+
+func main() {
+	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, "hello world")
+	})
+	http.ListenAndServe(":0", nil)
+}

cmd/dlv/dlv_test.go

@@ -40,7 +40,7 @@ func TestMain(m *testing.M) {
 			}
 		}
 	}
-	os.Exit(m.Run())
+	os.Exit(protest.RunTestsWithFixtures(m))
 }
 
 func assertNoError(err error, t testing.TB, s string) {
@@ -252,6 +252,50 @@ func TestContinue(t *testing.T) {
 	cmd.Wait()
 }
 
+// TestChildProcessExitWhenNoDebugInfo verifies that the child process exits when dlv launch the binary without debug info
+func TestChildProcessExitWhenNoDebugInfo(t *testing.T) {
+	if runtime.GOOS == "darwin" {
+		t.Skip("test skipped on darwin, see https://github.com/go-delve/delve/pull/2018 for details")
+	}
+
+	if _, err := exec.LookPath("ps"); err != nil {
+		t.Skip("test skipped, `ps` not found")
+	}
+
+	dlvbin, tmpdir := getDlvBin(t)
+	defer os.RemoveAll(tmpdir)
+
+	fix := protest.BuildFixture("http_server", protest.LinkStrip)
+
+	// dlv exec the binary file and expect error.
+	if _, err := exec.Command(dlvbin, "exec", fix.Path).CombinedOutput(); err == nil {
+		t.Fatalf("Expected err when launching the binary without debug info, but got nil")
+	}
+
+	// search the running process named fix.Name
+	cmd := exec.Command("ps", "-aux")
+	stdout, err := cmd.StdoutPipe()
+	assertNoError(err, t, "stderr pipe")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("`ps -aux` failed: %v", err)
+	}
+
+	var foundFlag bool
+	scan := bufio.NewScanner(stdout)
+	for scan.Scan() {
+		t.Log(scan.Text())
+		if strings.Contains(scan.Text(), fix.Name) {
+			foundFlag = true
+			break
+		}
+	}
+	cmd.Wait()
+
+	if foundFlag {
+		t.Fatalf("Expected child process exited, but found it running")
+	}
+}
+
 func checkAutogenDoc(t *testing.T, filename, gencommand string, generated []byte) {
 	saved := slurpFile(t, filepath.Join(projectRoot(), filename))
 

pkg/proc/native/proc_darwin.go

@@ -61,6 +61,11 @@ func Launch(cmd []string, wd string, foreground bool, _ []string, _ string) (*pr
 	argvSlice = append(argvSlice, nil)
 
 	dbp := newProcess(0)
+	defer func() {
+		if err != nil && dbp.pid != 0 {
+			_ = dbp.Detach(true)
+		}
+	}()
 	var pid int
 	dbp.execPtraceFunc(func() {
 		ret := C.fork_exec(argv0, &argvSlice[0], C.int(len(argvSlice)),

pkg/proc/native/proc_freebsd.go

@@ -56,6 +56,11 @@ func Launch(cmd []string, wd string, foreground bool, debugInfoDirs []string, tt
 	}
 
 	dbp := newProcess(0)
+	defer func() {
+		if err != nil && dbp.pid != 0 {
+			_ = dbp.Detach(true)
+		}
+	}()
 	dbp.execPtraceFunc(func() {
 		process = exec.Command(cmd[0])
 		process.Args = cmd

pkg/proc/native/proc_linux.go

@@ -62,6 +62,11 @@ func Launch(cmd []string, wd string, foreground bool, debugInfoDirs []string, tt
 	}
 
 	dbp := newProcess(0)
+	defer func() {
+		if err != nil && dbp.pid != 0 {
+			_ = dbp.Detach(true)
+		}
+	}()
 	dbp.execPtraceFunc(func() {
 		process = exec.Command(cmd[0])
 		process.Args = cmd
@@ -126,7 +131,7 @@ func Attach(pid int, debugInfoDirs []string) (*proc.Target, error) {
 
 	tgt, err := dbp.initialize(execPath, debugInfoDirs)
 	if err != nil {
-		dbp.Detach(false)
+		_ = dbp.Detach(false)
 		return nil, err
 	}
 
@@ -162,7 +167,7 @@ func initialize(dbp *nativeProcess) error {
 		}
 		comm = match[1]
 	}
-	dbp.os.comm = strings.Replace(string(comm), "%", "%%", -1)
+	dbp.os.comm = strings.ReplaceAll(string(comm), "%", "%%")
 	return nil
 }
 
@@ -362,8 +367,7 @@ func (dbp *nativeProcess) trapWaitInternal(pid int, options trapWaitOptions) (*n
 			th.os.delayedSignal = int(status.StopSignal())
 			th.os.running = false
 			return th, nil
-		} else {
-			if err := th.resumeWithSig(int(status.StopSignal())); err != nil {
+		} else if err := th.resumeWithSig(int(status.StopSignal())); err != nil {
 			if err == sys.ESRCH {
 				dbp.postExit()
 				return nil, proc.ErrProcessExited{Pid: dbp.pid}
@@ -371,7 +375,6 @@ func (dbp *nativeProcess) trapWaitInternal(pid int, options trapWaitOptions) (*n
 			return nil, err
 		}
 	}
-	}
 }
 
 func status(pid int, comm string) rune {
@@ -391,7 +394,7 @@ func status(pid int, comm string) rune {
 	// The name of the task is the base name of the executable for this process limited to TASK_COMM_LEN characters
 	// Since both parenthesis and spaces can appear inside the name of the task and no escaping happens we need to read the name of the executable first
 	// See: include/linux/sched.c:315 and include/linux/sched.c:1510
-	fmt.Fscanf(rd, "%d ("+comm+")  %c", &p, &state)
+	_, _ = fmt.Fscanf(rd, "%d ("+comm+")  %c", &p, &state)
 	return state
 }
 
@@ -545,7 +548,7 @@ func (dbp *nativeProcess) detach(kill bool) error {
 	// SIGCONT it if it is.
 	time.Sleep(50 * time.Millisecond)
 	if s := status(dbp.pid, dbp.os.comm); s == 'T' {
-		sys.Kill(dbp.pid, sys.SIGCONT)
+		_ = sys.Kill(dbp.pid, sys.SIGCONT)
 	}
 	return nil
 }