terminal,service: auto-continue during next and step (#448)

* proc: bugfix: StepInto can not function when temp bps exist

* terminal,service: auto-continue during next and step

Make dlv call continue automatically when a breakpoint is hit on a
different goroutine during a next or step operation.
Added API hooks to implement the other solution to this problem (cancel
the next/step operation if a different breakpoint is hit).

Fixes #387

_fixtures/issue387.go

@@ -0,0 +1,21 @@
+package main
+
+import (
+	"fmt"
+	"sync"
+)
+
+func dostuff(id int, wg *sync.WaitGroup) {
+	fmt.Println("goroutine:", id)
+	fmt.Println("goroutine:", id)
+	wg.Done()
+}
+
+func main() {
+	var wg sync.WaitGroup
+	wg.Add(10)
+	for i := 0; i < 10; i++ {
+		go dostuff(i, &wg)
+	}
+	wg.Wait()
+}

proc/proc.go

@@ -285,7 +285,7 @@ func (dbp *Process) Next() (err error) {
 		case GoroutineExitingError:
 			goroutineExiting = t.goid == g.ID
 		default:
-			dbp.clearTempBreakpoints()
+			dbp.ClearTempBreakpoints()
 			return
 		}
 	}
@@ -385,7 +385,7 @@ func (dbp *Process) Continue() error {
 			}
 			return dbp.conditionErrors()
 		case dbp.CurrentThread.onTriggeredTempBreakpoint():
-			err := dbp.clearTempBreakpoints()
+			err := dbp.ClearTempBreakpoints()
 			if err != nil {
 				return err
 			}
@@ -396,7 +396,7 @@ func (dbp *Process) Continue() error {
 				return err
 			}
 			if onNextGoroutine {
-				err := dbp.clearTempBreakpoints()
+				err := dbp.ClearTempBreakpoints()
 				if err != nil {
 					return err
 				}
@@ -484,6 +484,11 @@ func (dbp *Process) Step() (err error) {
 
 // StepInto sets a temp breakpoint after the prologue of fn and calls Continue
 func (dbp *Process) StepInto(fn *gosym.Func) error {
+	for i := range dbp.Breakpoints {
+		if dbp.Breakpoints[i].Temp {
+			return fmt.Errorf("next while nexting")
+		}
+	}
 	pc, _ := dbp.FirstPCAfterPrologue(fn, false)
 	if _, err := dbp.SetTempBreakpoint(pc); err != nil {
 		return err
@@ -754,7 +759,7 @@ func initializeDebugProcess(dbp *Process, path string, attach bool) (*Process, e
 	return dbp, nil
 }
 
-func (dbp *Process) clearTempBreakpoints() error {
+func (dbp *Process) ClearTempBreakpoints() error {
 	for _, bp := range dbp.Breakpoints {
 		if !bp.Temp {
 			continue

service/api/types.go

@@ -18,6 +18,11 @@ type DebuggerState struct {
 	SelectedGoroutine *Goroutine `json:"currentGoroutine,omitempty"`
 	// List of all the process threads
 	Threads []*Thread
+	// NextInProgress indicates that a next or step operation was interrupted by another breakpoint
+	// or a manual stop and is waiting to complete.
+	// While NextInProgress is set further requests for next or step may be rejected.
+	// Either execute continue until NextInProgress is false or call CancelNext
+	NextInProgress bool
 	// Exited indicates whether the debugged process has exited.
 	Exited     bool `json:"exited"`
 	ExitStatus int  `json:"exitStatus"`

service/client.go

@@ -49,6 +49,8 @@ type Client interface {
 	// Allows user to update an existing breakpoint for example to change the information
 	// retrieved when the breakpoint is hit or to change, add or remove the break condition
 	AmendBreakpoint(*api.Breakpoint) error
+	// Cancels a Next or Step call that was interrupted by a manual stop or by another breakpoint
+	CancelNext() error
 
 	// ListThreads lists all threads.
 	ListThreads() ([]*api.Thread, error)

service/debugger/debugger.go

@@ -163,6 +163,13 @@ func (d *Debugger) state() (*api.DebuggerState, error) {
 		}
 	}
 
+	for _, bp := range d.process.Breakpoints {
+		if bp.Temp {
+			state.NextInProgress = true
+			break
+		}
+	}
+
 	return state, nil
 }
 
@@ -243,6 +250,10 @@ func (d *Debugger) AmendBreakpoint(amend *api.Breakpoint) error {
 	return copyBreakpointInfo(original, amend)
 }
 
+func (d *Debugger) CancelNext() error {
+	return d.process.ClearTempBreakpoints()
+}
+
 func copyBreakpointInfo(bp *proc.Breakpoint, requested *api.Breakpoint) (err error) {
 	bp.Name = requested.Name
 	bp.Tracepoint = requested.Tracepoint

service/rpc1/client.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"net/rpc"
 	"net/rpc/jsonrpc"
+	"errors"
 
 	"github.com/derekparker/delve/service/api"
 	"sync"
@@ -19,6 +20,8 @@ type RPCClient struct {
 	haltReq    bool
 }
 
+var unsupportedApiError = errors.New("unsupported")
+
 // NewClient creates a new RPCClient.
 func NewClient(addr string) *RPCClient {
 	client, err := jsonrpc.Dial("tcp", addr)
@@ -186,6 +189,10 @@ func (c *RPCClient) AmendBreakpoint(bp *api.Breakpoint) error {
 	return err
 }
 
+func (c *RPCClient) CancelNext() error {
+	return unsupportedApiError
+}
+
 func (c *RPCClient) ListThreads() ([]*api.Thread, error) {
 	var threads []*api.Thread
 	err := c.call("ListThreads", nil, &threads)

service/rpc2/client.go

@@ -178,6 +178,11 @@ func (c *RPCClient) AmendBreakpoint(bp *api.Breakpoint) error {
 	return err
 }
 
+func (c *RPCClient) CancelNext() error {
+	var out CancelNextOut
+	return c.call("CancelNext", CancelNextIn{}, &out)
+}
+
 func (c *RPCClient) ListThreads() ([]*api.Thread, error) {
 	var out ListThreadsOut
 	err := c.call("ListThreads", ListThreadsIn{}, &out)

service/rpc2/server.go

@@ -315,6 +315,16 @@ func (s *RPCServer) AmendBreakpoint(arg AmendBreakpointIn, out *AmendBreakpointO
 	return s.debugger.AmendBreakpoint(&arg.Breakpoint)
 }
 
+type CancelNextIn struct {
+}
+
+type CancelNextOut struct {
+}
+
+func (s *RPCServer) CancelNext(arg CancelNextIn, out *CancelNextOut) error {
+	return s.debugger.CancelNext()
+}
+
 type ListThreadsIn struct {
 }
 

terminal/command.go

@@ -449,14 +449,35 @@ func cont(t *Term, ctx callContext, args string) error {
 	return nil
 }
 
+func continueUntilCompleteNext(t *Term, state *api.DebuggerState, op string) error {
+	if !state.NextInProgress {
+		printfile(t, state.CurrentThread.File, state.CurrentThread.Line, true)
+		return nil
+	}
+	for {
+		stateChan := t.client.Continue()
+		var state *api.DebuggerState
+		for state = range stateChan {
+			if state.Err != nil {
+				return state.Err
+			}
+			printcontext(t, state)
+		}
+		if !state.NextInProgress {
+			printfile(t, state.CurrentThread.File, state.CurrentThread.Line, true)
+			return nil
+		}
+		fmt.Printf("\tbreakpoint hit during %s, continuing...\n", op)
+	}
+}
+
 func step(t *Term, ctx callContext, args string) error {
 	state, err := t.client.Step()
 	if err != nil {
 		return err
 	}
 	printcontext(t, state)
-	printfile(t, state.CurrentThread.File, state.CurrentThread.Line, true)
-	return nil
+	return continueUntilCompleteNext(t, state, "step")
 }
 
 func stepInstruction(t *Term, ctx callContext, args string) error {
@@ -475,8 +496,7 @@ func next(t *Term, ctx callContext, args string) error {
 		return err
 	}
 	printcontext(t, state)
-	printfile(t, state.CurrentThread.File, state.CurrentThread.Line, true)
-	return nil
+	return continueUntilCompleteNext(t, state, "next")
 }
 
 func clear(t *Term, ctx callContext, args string) error {

terminal/command_test.go

@@ -71,6 +71,7 @@ func (ft *FakeTerminal) AssertExecError(cmdstr, tgterr string) {
 }
 
 func withTestTerminal(name string, t testing.TB, fn func(*FakeTerminal)) {
+	os.Setenv("TERM", "dumb")
 	listener, err := net.Listen("tcp", "localhost:0")
 	if err != nil {
 		t.Fatalf("couldn't start listener: %s\n", err)
@@ -398,3 +399,54 @@ func TestOnPrefixLocals(t *testing.T) {
 		}
 	})
 }
+
+
+func countOccourences(s string, needle string) int {
+	count := 0
+	for {
+		idx := strings.Index(s, needle)
+		if idx < 0 {
+			break
+		}
+		count++
+		s = s[idx+len(needle):]
+	}
+	return count
+}
+
+func TestIssue387(t *testing.T) {
+	// a breakpoint triggering during a 'next' operation will interrupt it
+	withTestTerminal("issue387", t, func(term *FakeTerminal) {
+		breakpointHitCount := 0
+		term.MustExec("break dostuff")
+		for {
+			outstr, err := term.Exec("continue")
+			breakpointHitCount += countOccourences(outstr, "issue387.go:8")
+			t.Log(outstr)
+			if err != nil {
+				if strings.Index(err.Error(), "exited") < 0 {
+					t.Fatalf("Unexpected error executing 'continue': %v", err)
+				}
+				break
+			}
+
+			pos := 9
+
+			for {
+				outstr = term.MustExec("next")
+				breakpointHitCount += countOccourences(outstr, "issue387.go:8")
+				t.Log(outstr)
+				if countOccourences(outstr, fmt.Sprintf("issue387.go:%d", pos)) == 0 {
+					t.Fatalf("did not continue to expected position %d", pos)
+				}
+				pos++
+				if pos > 11 {
+					break
+				}
+			}
+		}
+		if breakpointHitCount != 10 {
+			t.Fatalf("Breakpoint hit wrong number of times, expected 10 got %d", breakpointHitCount)
+		}
+	})
+}