proc/native: make sure debugged executable can be deleted on windows (#890)

* proc/native: make sure debugged executable can be deleted on windows

Delve opens debugged executable to read binary info it
contains, but it never closes the file. Windows will not
let you delete file that is opened. So close Process.bi
in Process.postExit, and actually call Process.postExit
from windows Process.Kill.

Also Windows sends some debugging events
(EXIT_PROCESS_DEBUG_EVENT event in particular) after Delve
calls TerminateProcess. The events need to be consumed by
debugger before debugged process will be released by
Windows. So call Process.waitForDebugEvent after
TerminateProcess in Process.Kill.

Fixes #398

* cmd/dlv: make TestIssue398 pass on darwin

* cmd/dlv: add comment for TestIssue398

* proc/native: wait for debuggee to exit before returning from windows Process.Kill

* proc/native: close process handle before returning from windows killProcess

* proc/native: remove not used Process.Process

cmd/dlv/dlv_test.go

@@ -2,7 +2,10 @@ package main
 
 import (
 	"bufio"
+	"bytes"
 	"flag"
+	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -96,3 +99,61 @@ func TestBuild(t *testing.T) {
 	client.Detach(true)
 	cmd.Wait()
 }
+
+func testIssue398(t *testing.T, dlvbin string, cmds []string) (stdout, stderr []byte) {
+	var stdoutBuf, stderrBuf bytes.Buffer
+	buildtestdir := filepath.Join(protest.FindFixturesDir(), "buildtest")
+	cmd := exec.Command(dlvbin, "debug")
+	cmd.Dir = buildtestdir
+	stdin, err := cmd.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	cmd.Stdout = &stdoutBuf
+	cmd.Stderr = &stderrBuf
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	for _, c := range cmds {
+		fmt.Fprintf(stdin, "%s\n", c)
+	}
+	// ignore "dlv debug" command error, it returns
+	// errors even after successful debug session.
+	cmd.Wait()
+	stdout, stderr = stdoutBuf.Bytes(), stderrBuf.Bytes()
+
+	debugbin := filepath.Join(buildtestdir, "debug")
+	_, err = os.Stat(debugbin)
+	if err == nil {
+		t.Errorf("running %q: file %v was not deleted\nstdout is %q, stderr is %q", cmds, debugbin, stdout, stderr)
+		return
+	}
+	if !os.IsNotExist(err) {
+		t.Errorf("running %q: %v\nstdout is %q, stderr is %q", cmds, err, stdout, stderr)
+		return
+	}
+	return
+}
+
+// TestIssue398 verifies that the debug executable is removed after exit.
+func TestIssue398(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "TestIssue398")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	dlvbin := filepath.Join(tmpdir, "dlv.exe")
+	out, err := exec.Command("go", "build", "-o", dlvbin, "github.com/derekparker/delve/cmd/dlv").CombinedOutput()
+	if err != nil {
+		t.Fatalf("go build -o %v github.com/derekparker/delve/cmd/dlv: %v\n%s", dlvbin, err, string(out))
+	}
+
+	testIssue398(t, dlvbin, []string{"exit"})
+
+	const hello = "hello world!"
+	stdout, _ := testIssue398(t, dlvbin, []string{"continue", "exit"})
+	if !strings.Contains(string(stdout), hello) {
+		t.Errorf("stdout %q should contain %q", stdout, hello)
+	}
+}

pkg/proc/native/proc.go

@@ -3,7 +3,6 @@ package native
 import (
 	"fmt"
 	"go/ast"
-	"os"
 	"runtime"
 	"sync"
 
@@ -15,7 +14,6 @@ import (
 type Process struct {
 	bi  proc.BinaryInfo
 	pid int // Process Pid
-	Process *os.Process // Pointer to process struct for the actual process we are debugging
 
 	// Breakpoint table, holds information on breakpoints.
 	// Maps instruction address to Breakpoint struct.
@@ -388,13 +386,7 @@ func (dbp *Process) FindBreakpoint(pc uint64) (*proc.Breakpoint, bool) {
 
 // Returns a new Process struct.
 func initializeDebugProcess(dbp *Process, path string) (*Process, error) {
-	process, err := os.FindProcess(dbp.pid)
-	if err != nil {
-		return nil, err
-	}
-
-	dbp.Process = process
-	err = dbp.LoadInformation(path)
+	err := dbp.LoadInformation(path)
 	if err != nil {
 		return nil, err
 	}
@@ -461,6 +453,7 @@ func (dbp *Process) postExit() {
 	dbp.exited = true
 	close(dbp.ptraceChan)
 	close(dbp.ptraceDoneChan)
+	dbp.bi.Close()
 }
 
 func (dbp *Process) writeSoftwareBreakpoint(thread *Thread, addr uint64) error {

pkg/proc/native/proc_windows.go

@@ -173,11 +173,25 @@ func (dbp *Process) Kill() error {
 	if !dbp.threads[dbp.pid].Stopped() {
 		return errors.New("process must be stopped in order to kill it")
 	}
+
+	p, err := os.FindProcess(dbp.pid)
+	if err != nil {
+		return err
+	}
+	defer p.Release()
+
 	// TODO: Should not have to ignore failures here,
 	// but some tests appear to Kill twice causing
 	// this to fail on second attempt.
 	_ = syscall.TerminateProcess(dbp.os.hProcess, 1)
-	dbp.exited = true
+
+	dbp.execPtraceFunc(func() {
+		dbp.waitForDebugEvent(waitBlocking)
+	})
+
+	p.Wait()
+
+	dbp.postExit()
 	return nil
 }
 
@@ -459,5 +473,7 @@ func killProcess(pid int) error {
 	if err != nil {
 		return err
 	}
+	defer p.Release()
+
 	return p.Kill()
 }