proc/linux: do not route signals to threads while stopping (#1752)

* proc/linux: do not route signals to threads while stopping

While we are trying to stop the process we should not route signals
sent to threads because that will result in threads being resumed.
Also keep better track of which threads are stopped.

This fixes an incompatibility with Go 1.14, which sends a lot of
signals to its threads to implement non-cooperative preemption,
resulting in Delve hanging waiting for an already-stopped thread to
stop.

In principle however this bug has nothing to do with Go 1.14 and could
manifest in any instance of high signal pressure.

* Makefile: discard stderr of "go list"

In module mode "go" will print messages about downloading modules to
stderr, we shouldn't confuse them for the real command output.
This commit is contained in:
Alessandro Arzilli 2019-11-12 15:58:54 +01:00 committed by Derek Parker
parent 79143468ea
commit 4fc8528997
4 changed files with 27 additions and 10 deletions

@ -319,12 +319,23 @@ func (dbp *Process) trapWaitInternal(pid int, halt bool) (*Thread, error) {
} }
// TODO(dp) alert user about unexpected signals here. // TODO(dp) alert user about unexpected signals here.
if err := th.resumeWithSig(int(status.StopSignal())); err != nil { if halt && !th.os.running {
if err == sys.ESRCH { // We are trying to stop the process, queue this signal to be delivered
dbp.postExit() // to the thread when we resume.
return nil, proc.ErrProcessExited{Pid: dbp.pid} // Do not do this for threads that were running because we sent them a
// STOP signal and we need to observe it so we don't mistakenly deliver
// it later.
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
} }
return nil, err
} }
} }
} }
@ -429,6 +440,9 @@ func (dbp *Process) stop(trapthread *Thread) (err error) {
if err := th.stop(); err != nil { if err := th.stop(); err != nil {
return dbp.exitGuard(err) return dbp.exitGuard(err)
} }
} else {
// Thread is already in a trace stop but we didn't get the notification yet.
th.os.running = false
} }
} }

@ -16,8 +16,9 @@ type WaitStatus sys.WaitStatus
// OSSpecificDetails hold Linux specific // OSSpecificDetails hold Linux specific
// process details. // process details.
type OSSpecificDetails struct { type OSSpecificDetails struct {
registers sys.PtraceRegs delayedSignal int
running bool registers sys.PtraceRegs
running bool
} }
func (t *Thread) stop() (err error) { func (t *Thread) stop() (err error) {
@ -37,7 +38,9 @@ func (t *Thread) Stopped() bool {
} }
func (t *Thread) resume() error { func (t *Thread) resume() error {
return t.resumeWithSig(0) sig := t.os.delayedSignal
t.os.delayedSignal = 0
return t.resumeWithSig(sig)
} }
func (t *Thread) resumeWithSig(sig int) (err error) { func (t *Thread) resumeWithSig(sig int) (err error) {

@ -953,7 +953,7 @@ func TestStacktraceGoroutine(t *testing.T) {
if locations[i].Call.Fn != nil { if locations[i].Call.Fn != nil {
name = locations[i].Call.Fn.Name name = locations[i].Call.Fn.Name
} }
t.Logf("\t%s:%d %s (%#x)\n", locations[i].Call.File, locations[i].Call.Line, name, locations[i].Current.PC) t.Logf("\t%s:%d %s (%#x) %x %v\n", locations[i].Call.File, locations[i].Call.Line, name, locations[i].Current.PC, locations[i].FrameOffset(), locations[i].SystemStack)
} }
} }
} }

@ -187,7 +187,7 @@ func quotemaybe(args []string) []string {
func getoutput(cmd string, args ...interface{}) string { func getoutput(cmd string, args ...interface{}) string {
x := exec.Command(cmd, strflatten(args)...) x := exec.Command(cmd, strflatten(args)...)
x.Env = os.Environ() x.Env = os.Environ()
out, err := x.CombinedOutput() out, err := x.Output()
if err != nil { if err != nil {
log.Fatal(err) log.Fatal(err)
} }