cmd/dlv: print out message with stack trace when breakpoint is hit but has no waiting client (#3632)

* Print out message and dump stack on pause

* Fix test

* Move the logic to debugger layer

* Remove unused fields

* Do not use defer to get state

* move channel to connection

* remove lock on isClosed

* Use mutex

* Remove unwanted changes

cmd/dlv/dlv_test.go

@@ -251,6 +251,35 @@ func TestOutput(t *testing.T) {
 	}
 }
 
+// TestUnattendedBreakpoint tests whether dlv will print a message to stderr when the client that sends continue is disconnected
+// or not.
+func TestUnattendedBreakpoint(t *testing.T) {
+	const listenAddr = "127.0.0.1:40573"
+
+	fixturePath := filepath.Join(protest.FindFixturesDir(), "panic.go")
+	cmd := exec.Command(getDlvBin(t), "debug", "--continue", "--headless", "--accept-multiclient", "--listen", listenAddr, fixturePath)
+	stderr, err := cmd.StderrPipe()
+	assertNoError(err, t, "stdout pipe")
+	defer stderr.Close()
+
+	assertNoError(cmd.Start(), t, "start headless instance")
+
+	scan := bufio.NewScanner(stderr)
+	for scan.Scan() {
+		t.Log(scan.Text())
+		if strings.Contains(scan.Text(), "execution is paused because your program is panicking") {
+			break
+		}
+	}
+
+	// and detach from and kill the headless instance
+	client := rpc2.NewClient(listenAddr)
+	if err := client.Detach(true); err != nil {
+		t.Fatalf("error detaching from headless instance: %v", err)
+	}
+	cmd.Wait()
+}
+
 // TestContinue verifies that the debugged executable starts immediately with --continue
 func TestContinue(t *testing.T) {
 	const listenAddr = "127.0.0.1:40573"

service/dap/server.go

@@ -181,14 +181,23 @@ type Config struct {
 }
 
 type connection struct {
-	mu     sync.Mutex
+	mu         sync.Mutex
-	closed bool
+	closed     bool
+	closedChan chan struct{}
 	io.ReadWriteCloser
 }
 
+func newConnection(conn io.ReadWriteCloser) *connection {
+	return &connection{ReadWriteCloser: conn, closedChan: make(chan struct{})}
+}
+
 func (c *connection) Close() error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
+
+	if !c.closed {
+		close(c.closedChan)
+	}
 	c.closed = true
 	return c.ReadWriteCloser.Close()
 }
@@ -335,7 +344,7 @@ func NewSession(conn io.ReadWriteCloser, config *Config, debugger *debugger.Debu
 	return &Session{
 		config:            config,
 		id:                sessionCount,
-		conn:              &connection{ReadWriteCloser: conn},
+		conn:              newConnection(conn),
 		stackFrameHandles: newHandlesMap(),
 		variableHandles:   newVariablesHandlesMap(),
 		args:              defaultArgs,
@@ -1366,7 +1375,7 @@ func (s *Session) halt() (*api.DebuggerState, error) {
 	s.config.log.Debug("halting")
 	// Only send a halt request if the debuggee is running.
 	if s.debugger.IsRunning() {
-		return s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil)
+		return s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil, nil)
 	}
 	s.config.log.Debug("process not running")
 	return s.debugger.State(false)
@@ -2048,7 +2057,7 @@ func (s *Session) stoppedOnBreakpointGoroutineID(state *api.DebuggerState) (int6
 // due to an error, so the server is ready to receive new requests.
 func (s *Session) stepUntilStopAndNotify(command string, threadId int, granularity dap.SteppingGranularity, allowNextStateChange *syncflag) {
 	defer allowNextStateChange.raise()
-	_, err := s.debugger.Command(&api.DebuggerCommand{Name: api.SwitchGoroutine, GoroutineID: int64(threadId)}, nil)
+	_, err := s.debugger.Command(&api.DebuggerCommand{Name: api.SwitchGoroutine, GoroutineID: int64(threadId)}, nil, s.conn.closedChan)
 	if err != nil {
 		s.config.log.Errorf("Error switching goroutines while stepping: %v", err)
 		// If we encounter an error, we will have to send a stopped event
@@ -2914,7 +2923,7 @@ func (s *Session) doCall(goid, frame int, expr string) (*api.DebuggerState, []*p
 		Expr:                 expr,
 		UnsafeCall:           false,
 		GoroutineID:          int64(goid),
-	}, nil)
+	}, nil, s.conn.closedChan)
 	if processExited(state, err) {
 		s.preTerminatedWG.Wait()
 		e := &dap.TerminatedEvent{Event: *newEvent("terminated")}
@@ -3645,7 +3654,7 @@ func (s *Session) resumeOnce(command string, allowNextStateChange *syncflag) (bo
 		state, err := s.debugger.State(false)
 		return false, state, err
 	}
-	state, err := s.debugger.Command(&api.DebuggerCommand{Name: command}, asyncSetupDone)
+	state, err := s.debugger.Command(&api.DebuggerCommand{Name: command}, asyncSetupDone, s.conn.closedChan)
 	return true, state, err
 }
 

service/dap/server_test.go

@@ -6736,7 +6736,7 @@ func launchDebuggerWithTargetRunning(t *testing.T, fixture string) (*protest.Fix
 	var err error
 	go func() {
 		t.Helper()
-		_, err = dbg.Command(&api.DebuggerCommand{Name: api.Continue}, running)
+		_, err = dbg.Command(&api.DebuggerCommand{Name: api.Continue}, running, nil)
 		select {
 		case <-running:
 		default:
@@ -6934,7 +6934,7 @@ func (s *MultiClientCloseServerMock) stop(t *testing.T) {
 	// they are part of dap.Session.
 	// We must take it down manually as if we are in rpccommon::ServerImpl::Stop.
 	if s.debugger.IsRunning() {
-		s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil)
+		s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil, nil)
 	}
 	s.debugger.Detach(true)
 }

service/debugger/debugger.go

@@ -1159,9 +1159,7 @@ func (d *Debugger) IsRunning() bool {
 }
 
 // Command handles commands which control the debugger lifecycle
-func (d *Debugger) Command(command *api.DebuggerCommand, resumeNotify chan struct{}) (*api.DebuggerState, error) {
+func (d *Debugger) Command(command *api.DebuggerCommand, resumeNotify chan struct{}, clientStatusCh chan struct{}) (state *api.DebuggerState, err error) {
-	var err error
-
 	if command.Name == api.Halt {
 		// RequestManualStop does not invoke any ptrace syscalls, so it's safe to
 		// access the process directly.
@@ -1338,6 +1336,8 @@ func (d *Debugger) Command(command *api.DebuggerCommand, resumeNotify chan struc
 		bp.Disabled = true
 		d.amendBreakpoint(bp)
 	}
+
+	d.maybePrintUnattendedBreakpointWarning(state.CurrentThread, clientStatusCh)
 	return state, err
 }
 
@@ -1785,7 +1785,10 @@ func (d *Debugger) GroupGoroutines(gs []*proc.G, group *api.GoroutineGroupingOpt
 func (d *Debugger) Stacktrace(goroutineID int64, depth int, opts api.StacktraceOptions) ([]proc.Stackframe, error) {
 	d.targetMutex.Lock()
 	defer d.targetMutex.Unlock()
+	return d.stacktrace(goroutineID, depth, opts)
+}
 
+func (d *Debugger) stacktrace(goroutineID int64, depth int, opts api.StacktraceOptions) ([]proc.Stackframe, error) {
 	if _, err := d.target.Valid(); err != nil {
 		return nil, err
 	}
@@ -2426,3 +2429,51 @@ var attachErrorMessage = attachErrorMessageDefault
 func attachErrorMessageDefault(pid int, err error) error {
 	return fmt.Errorf("could not attach to pid %d: %s", pid, err)
 }
+
+func (d *Debugger) maybePrintUnattendedBreakpointWarning(currentThread *api.Thread, clientStatusCh <-chan struct{}) {
+	select {
+	case <-clientStatusCh:
+		// the channel will be closed if the client that sends the command has left
+		// i.e. closed the connection.
+	default:
+		return
+	}
+
+	const defaultStackTraceDepth = 50
+	frames, err := d.stacktrace(currentThread.GoroutineID, defaultStackTraceDepth, 0)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "err", err)
+		return
+	}
+
+	apiFrames, err := d.convertStacktrace(frames, nil)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "err", err)
+		return
+	}
+
+	bp := currentThread.Breakpoint
+	if bp == nil {
+		fmt.Fprintln(os.Stderr, "bp", bp)
+		return
+	}
+
+	switch bp.Name {
+	case proc.FatalThrow, proc.UnrecoveredPanic:
+		fmt.Fprintln(os.Stderr, "\n** execution is paused because your program is panicking **")
+	default:
+		fmt.Fprintln(os.Stderr, "\n** execution is paused because a breakpoint is hit **")
+	}
+
+	fmt.Fprintf(os.Stderr, "To continue the execution please connect your client to the debugger.")
+	fmt.Fprintln(os.Stderr, "\nStack trace:")
+
+	formatPathFunc := func(s string) string {
+		return s
+	}
+	includeFunc := func(f api.Stackframe) bool {
+		// todo(fata): do not include the final panic/fatal function if bp.Name is fatalthrow/panic
+		return true
+	}
+	api.PrintStack(formatPathFunc, os.Stderr, apiFrames, "", false, api.StackTraceColors{}, includeFunc)
+}

service/rpc1/server.go

@@ -56,7 +56,7 @@ func (s *RPCServer) State(arg interface{}, state *api.DebuggerState) error {
 }
 
 func (s *RPCServer) Command(command *api.DebuggerCommand, cb service.RPCCallback) {
-	st, err := s.debugger.Command(command, cb.SetupDoneChan())
+	st, err := s.debugger.Command(command, cb.SetupDoneChan(), cb.DisconnectChan())
 	cb.Return(st, err)
 }
 

service/rpc2/server.go

@@ -127,7 +127,7 @@ type CommandOut struct {
 
 // Command interrupts, continues and steps through the program.
 func (s *RPCServer) Command(command api.DebuggerCommand, cb service.RPCCallback) {
-	st, err := s.debugger.Command(&command, cb.SetupDoneChan())
+	st, err := s.debugger.Command(&command, cb.SetupDoneChan(), cb.DisconnectChan())
 	if err != nil {
 		cb.Return(nil, err)
 		return

service/rpccallback.go

@@ -8,4 +8,8 @@ type RPCCallback interface {
 	// asynchronous method has completed setup and the server is ready to
 	// receive other requests.
 	SetupDoneChan() chan struct{}
+
+	// DisconnectChan returns a channel that should be clised to signal that
+	// the client that initially issued the command has been disconnected.
+	DisconnectChan() chan struct{}
 }

service/rpccommon/server.go

@@ -48,11 +48,12 @@ type ServerImpl struct {
 }
 
 type RPCCallback struct {
-	s         *ServerImpl
+	s              *ServerImpl
-	sending   *sync.Mutex
+	sending        *sync.Mutex
-	codec     rpc.ServerCodec
+	codec          rpc.ServerCodec
-	req       rpc.Request
+	req            rpc.Request
-	setupDone chan struct{}
+	setupDone      chan struct{}
+	disconnectChan chan struct{}
 }
 
 var _ service.RPCCallback = &RPCCallback{}
@@ -97,7 +98,7 @@ func (s *ServerImpl) Stop() error {
 		s.listener.Close()
 	}
 	if s.debugger.IsRunning() {
-		s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil)
+		s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil, nil)
 	}
 	kill := s.config.Debugger.AttachPid == 0
 	return s.debugger.Detach(kill)
@@ -277,7 +278,9 @@ func suitableMethods(rcvr interface{}, methods map[string]*methodType, log logfl
 }
 
 func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
+	clientDisconnectChan := make(chan struct{})
 	defer func() {
+		close(clientDisconnectChan)
 		if !s.config.AcceptMulti && s.config.DisconnectChan != nil {
 			close(s.config.DisconnectChan)
 		}
@@ -361,7 +364,7 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
 				s.log.Debugf("(async %d) <- %s(%T%s)", req.Seq, req.ServiceMethod, argv.Interface(), argvbytes)
 			}
 			function := mtype.method.Func
-			ctl := &RPCCallback{s, sending, codec, req, make(chan struct{})}
+			ctl := &RPCCallback{s, sending, codec, req, make(chan struct{}), clientDisconnectChan}
 			go func() {
 				defer func() {
 					if ierr := recover(); ierr != nil {
@@ -412,9 +415,28 @@ func (cb *RPCCallback) Return(out interface{}, err error) {
 		outbytes, _ := json.Marshal(out)
 		cb.s.log.Debugf("(async %d) -> %T%s error: %q", cb.req.Seq, out, outbytes, errmsg)
 	}
+
+	if cb.hasDisconnected() {
+		return
+	}
+
 	cb.s.sendResponse(cb.sending, &cb.req, &resp, out, cb.codec, errmsg)
 }
 
+func (cb *RPCCallback) DisconnectChan() chan struct{} {
+	return cb.disconnectChan
+}
+
+func (cb *RPCCallback) hasDisconnected() bool {
+	select {
+	case <-cb.disconnectChan:
+		return true
+	default:
+	}
+
+	return false
+}
+
 func (cb *RPCCallback) SetupDoneChan() chan struct{} {
 	return cb.setupDone
 }