From e28e3d30d26ab7ade78871dd7cd4a48850bc15f7 Mon Sep 17 00:00:00 2001 From: chainhelen Date: Thu, 14 May 2020 20:23:16 +0800 Subject: [PATCH] [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 | 13 ++++++++++ cmd/dlv/dlv_test.go | 46 ++++++++++++++++++++++++++++++++- pkg/proc/native/proc_darwin.go | 5 ++++ pkg/proc/native/proc_freebsd.go | 5 ++++ pkg/proc/native/proc_linux.go | 25 ++++++++++-------- 5 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 _fixtures/http_server.go diff --git a/_fixtures/http_server.go b/_fixtures/http_server.go new file mode 100644 index 00000000..4bb85e85 --- /dev/null +++ b/_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) +} diff --git a/cmd/dlv/dlv_test.go b/cmd/dlv/dlv_test.go index f0d02e01..ef49b582 100644 --- a/cmd/dlv/dlv_test.go +++ b/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)) diff --git a/pkg/proc/native/proc_darwin.go b/pkg/proc/native/proc_darwin.go index 95636462..ae7681cf 100644 --- a/pkg/proc/native/proc_darwin.go +++ b/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)), diff --git a/pkg/proc/native/proc_freebsd.go b/pkg/proc/native/proc_freebsd.go index 9e2c17ed..6ae123fd 100644 --- a/pkg/proc/native/proc_freebsd.go +++ b/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 diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go index 6a8ac855..1975753b 100644 --- a/pkg/proc/native/proc_linux.go +++ b/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,14 +367,12 @@ 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 { - if err == sys.ESRCH { - dbp.postExit() - return nil, proc.ErrProcessExited{Pid: dbp.pid} - } - return nil, err + } else if err := th.resumeWithSig(int(status.StopSignal())); err != nil { + if err == sys.ESRCH { + dbp.postExit() + return nil, proc.ErrProcessExited{Pid: dbp.pid} } + return nil, err } } } @@ -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 }