proc/*: remove Process.Running

Implementing proc.Process.Running in a thread safe way is complicated
and nothing actually uses it besides tests, so we are better off
rewriting the tests without Running and removing it.

In particular:

* The call to d.target.Running() in service/debugger/debugger.go
  (Restart) can never return true because that line executes while
  holding processMutex and all continue operations are also executed
  while holding processMutex.
* The call to dbp.Running() pkg/proc/native/proc.go (Detach) can never
  return true, because it's only called from
  debugger.(*Debugger).detach() which is also always called while
  holding processMutex.

Since some tests are hard to write correctly without Process.Running a
simpler interface, Process.NotifyResumed, is introduced.

Fixes #830

pkg/proc/core/core.go

@@ -304,8 +304,7 @@ func (p *Process) Pid() int {
 	return p.core.Pid
 }
 
-func (p *Process) Running() bool {
+func (p *Process) ResumeNotify(chan<- struct{}) {
-	return false
 }
 
 func (p *Process) SelectedGoroutine() *proc.G {

pkg/proc/gdbserial/gdbserver.go

@@ -490,8 +490,8 @@ func (p *Process) Exited() bool {
 	return p.exited
 }
 
-func (p *Process) Running() bool {
+func (p *Process) ResumeNotify(ch chan<- struct{}) {
-	return p.conn.running
+	p.conn.resumeChan = ch
 }
 
 func (p *Process) FindThread(threadID int) (proc.Thread, bool) {

pkg/proc/gdbserial/gdbserver_conn.go

@@ -23,7 +23,8 @@ type gdbConn struct {
 	inbuf  []byte
 	outbuf bytes.Buffer
 
-	running bool
+	running    bool
+	resumeChan chan<- struct{}
 
 	direction proc.Direction // direction of execution
 
@@ -543,6 +544,10 @@ func (conn *gdbConn) resume(sig uint8, tu *threadUpdater) (string, uint8, error)
 	defer func() {
 		conn.running = false
 	}()
+	if conn.resumeChan != nil {
+		close(conn.resumeChan)
+		conn.resumeChan = nil
+	}
 	return conn.waitForvContStop("resume", "-1", tu)
 }
 

pkg/proc/interface.go

@@ -52,8 +52,10 @@ type Checkpoint struct {
 // Info is an interface that provides general information on the target.
 type Info interface {
 	Pid() int
+	// ResumeNotify specifies a channel that will be closed the next time
+	// ContinueOnce finishes resuming the target.
+	ResumeNotify(chan<- struct{})
 	Exited() bool
-	Running() bool
 	BinInfo() *BinaryInfo
 
 	ThreadInfo

pkg/proc/native/proc.go

@@ -39,6 +39,7 @@ type Process struct {
 	firstStart                  bool
 	haltMu                      sync.Mutex
 	halt                        bool
+	resumeChan                  chan<- struct{}
 	exited                      bool
 	ptraceChan                  chan func()
 	ptraceDoneChan              chan interface{}
@@ -89,11 +90,6 @@ func (dbp *Process) Detach(kill bool) (err error) {
 		dbp.bi.Close()
 		return nil
 	}
-	if dbp.Running() {
-		if err = dbp.Halt(); err != nil {
-			return
-		}
-	}
 	if !kill {
 		// Clean up any breakpoints we've set.
 		for _, bp := range dbp.breakpoints {
@@ -124,15 +120,8 @@ func (dbp *Process) Exited() bool {
 	return dbp.exited
 }
 
-// Running returns whether the debugged
+func (dbp *Process) ResumeNotify(ch chan<- struct{}) {
-// process is currently executing.
+	dbp.resumeChan = ch
-func (dbp *Process) Running() bool {
-	for _, th := range dbp.threads {
-		if th.running {
-			return true
-		}
-	}
-	return false
 }
 
 func (dbp *Process) Pid() int {
@@ -275,6 +264,11 @@ func (dbp *Process) ContinueOnce() (proc.Thread, error) {
 		th.clearBreakpointState()
 	}
 
+	if dbp.resumeChan != nil {
+		close(dbp.resumeChan)
+		dbp.resumeChan = nil
+	}
+
 	trapthread, err := dbp.trapWait(-1)
 	if err != nil {
 		return nil, err

pkg/proc/proc_test.go

@@ -213,27 +213,22 @@ func TestHalt(t *testing.T) {
 		_, err := setFunctionBreakpoint(p, "main.loop")
 		assertNoError(err, t, "SetBreakpoint")
 		assertNoError(proc.Continue(p), t, "Continue")
-		if p.Running() {
-			t.Fatal("process still running")
-		}
 		if p, ok := p.(*native.Process); ok {
 			for _, th := range p.ThreadList() {
 				_, err := th.Registers(false)
 				assertNoError(err, t, "Registers")
 			}
 		}
+		resumeChan := make(chan struct{})
 		go func() {
-			for {
+			<-resumeChan
-				time.Sleep(100 * time.Millisecond)
+			time.Sleep(100 * time.Millisecond)
-				if p.Running() {
+			if err := p.RequestManualStop(); err != nil {
-					if err := p.RequestManualStop(); err != nil {
+				t.Fatal(err)
-						t.Fatal(err)
-					}
-					stopChan <- nil
-					return
-				}
 			}
+			stopChan <- nil
 		}()
+		p.ResumeNotify(resumeChan)
 		assertNoError(proc.Continue(p), t, "Continue")
 		<-stopChan
 		// Loop through threads and make sure they are all
@@ -601,9 +596,6 @@ func TestNextNetHTTP(t *testing.T) {
 	}
 	withTestProcess("testnextnethttp", t, func(p proc.Process, fixture protest.Fixture) {
 		go func() {
-			for !p.Running() {
-				time.Sleep(50 * time.Millisecond)
-			}
 			// Wait for program to start listening.
 			for {
 				conn, err := net.Dial("tcp", "localhost:9191")
@@ -1934,10 +1926,6 @@ func TestIssue462(t *testing.T) {
 	}
 	withTestProcess("testnextnethttp", t, func(p proc.Process, fixture protest.Fixture) {
 		go func() {
-			for !p.Running() {
-				time.Sleep(50 * time.Millisecond)
-			}
-
 			// Wait for program to start listening.
 			for {
 				conn, err := net.Dial("tcp", "localhost:9191")

pkg/proc/proc_unix_test.go

@@ -25,24 +25,21 @@ func TestIssue419(t *testing.T) {
 		_, err := setFunctionBreakpoint(p, "main.main")
 		assertNoError(err, t, "SetBreakpoint()")
 		assertNoError(proc.Continue(p), t, "Continue()")
+		resumeChan := make(chan struct{})
 		go func() {
-			for {
+			time.Sleep(500 * time.Millisecond)
-				time.Sleep(500 * time.Millisecond)
+			<-resumeChan
-				if p.Running() {
+			if p.Pid() <= 0 {
-					time.Sleep(2 * time.Second)
+				// if we don't stop the inferior the test will never finish
-					if p.Pid() <= 0 {
+				p.RequestManualStop()
-						// if we don't stop the inferior the test will never finish
+				p.Kill()
-						p.RequestManualStop()
+				t.Fatalf("Pid is zero or negative: %d", p.Pid())
-						p.Kill()
+				return
-						t.Fatalf("Pid is zero or negative: %d", p.Pid())
-						return
-					}
-					err := syscall.Kill(p.Pid(), syscall.SIGINT)
-					assertNoError(err, t, "syscall.Kill")
-					return
-				}
 			}
+			err := syscall.Kill(p.Pid(), syscall.SIGINT)
+			assertNoError(err, t, "syscall.Kill")
 		}()
+		p.ResumeNotify(resumeChan)
 		err = proc.Continue(p)
 		if _, exited := err.(proc.ProcessExitedError); !exited {
 			t.Fatalf("Unexpected error after Continue(): %v\n", err)

service/debugger/debugger.go

@@ -200,9 +200,6 @@ func (d *Debugger) Restart(pos string) ([]api.DiscardedBreakpoint, error) {
 	}
 
 	if !d.target.Exited() {
-		if d.target.Running() {
-			d.target.Halt()
-		}
 		// Ensure the process is in a PTRACE_STOP.
 		if err := stopProcess(d.ProcessPid()); err != nil {
 			return nil, err