mirror of
https://github.com/golang/go
synced 2024-09-04 23:44:16 +00:00
runtime: fix hangs in TestDebugCall*
This fixes a few different issues that led to hangs and general flakiness in the TestDebugCall* tests. 1. This fixes missing wake-ups in two error paths of the SIGTRAP signal handler. If the goroutine was in an unknown state, or if there was an unknown debug call status, we currently don't wake the injection coordinator. These are terminal states, so this resulted in a hang. 2. This adds a retry if the target goroutine is in a transient state that prevents us from injecting a call. The most common failure mode here is that the target goroutine is in _Grunnable, but this was previously masked because it deadlocked the test. 3. Related to 2, this switches the "ready" signal from the target goroutine from a blocking channel send to a non-blocking channel send. This makes it much less likely that we'll catch this goroutine while it's in the runtime performing that send. 4. This increases GOMAXPROCS from 2 to 8 during these tests. With the current setting of 2, we can have at most the non-preemptible goroutine we're injecting a call in to and the goroutine that's trying to make it exit. If anything else comes along, it can deadlock. One particular case I observed was in TestDebugCallGC, where runtime.GC() returns before the forEachP that prepares sweeping on all goroutines has finished. When this happens, the forEachP blocks on the non-preemptible loop, which means we now have at least three goroutines that need to run. Fixes #25519. Updates #29124. Change-Id: I7bc41dc0b865b7d0bb379cb654f9a1218bc37428 Reviewed-on: https://go-review.googlesource.com/c/154112 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
This commit is contained in:
parent
e167587dd6
commit
a27f3d5cfb
|
@ -33,11 +33,17 @@ func startDebugCallWorker(t *testing.T) (g *runtime.G, after func()) {
|
|||
skipUnderDebugger(t)
|
||||
|
||||
// This can deadlock if there aren't enough threads or if a GC
|
||||
// tries to interrupt an atomic loop (see issue #10958).
|
||||
ogomaxprocs := runtime.GOMAXPROCS(2)
|
||||
// tries to interrupt an atomic loop (see issue #10958). We
|
||||
// use 8 Ps so there's room for the debug call worker,
|
||||
// something that's trying to preempt the call worker, and the
|
||||
// goroutine that's trying to stop the call worker.
|
||||
ogomaxprocs := runtime.GOMAXPROCS(8)
|
||||
ogcpercent := debug.SetGCPercent(-1)
|
||||
|
||||
ready := make(chan *runtime.G)
|
||||
// ready is a buffered channel so debugCallWorker won't block
|
||||
// on sending to it. This makes it less likely we'll catch
|
||||
// debugCallWorker while it's in the runtime.
|
||||
ready := make(chan *runtime.G, 1)
|
||||
var stop uint32
|
||||
done := make(chan error)
|
||||
go debugCallWorker(ready, &stop, done)
|
||||
|
@ -67,6 +73,10 @@ func debugCallWorker(ready chan<- *runtime.G, stop *uint32, done chan<- error) {
|
|||
close(done)
|
||||
}
|
||||
|
||||
// Don't inline this function, since we want to test adjusting
|
||||
// pointers in the arguments.
|
||||
//
|
||||
//go:noinline
|
||||
func debugCallWorker2(stop *uint32, x *int) {
|
||||
for atomic.LoadUint32(stop) == 0 {
|
||||
// Strongly encourage x to live in a register so we
|
||||
|
@ -193,7 +203,7 @@ func TestDebugCallUnsafePoint(t *testing.T) {
|
|||
|
||||
// This can deadlock if there aren't enough threads or if a GC
|
||||
// tries to interrupt an atomic loop (see issue #10958).
|
||||
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))
|
||||
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(8))
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
|
||||
// Test that the runtime refuses call injection at unsafe points.
|
||||
|
@ -215,7 +225,7 @@ func TestDebugCallPanic(t *testing.T) {
|
|||
skipUnderDebugger(t)
|
||||
|
||||
// This can deadlock if there aren't enough threads.
|
||||
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))
|
||||
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(8))
|
||||
|
||||
ready := make(chan *runtime.G)
|
||||
var stop uint32
|
||||
|
|
|
@ -50,19 +50,31 @@ func InjectDebugCall(gp *g, fn, args interface{}, tkill func(tid int) error) (in
|
|||
h.gp = gp
|
||||
h.fv, h.argp, h.argSize = fv, argp, argSize
|
||||
h.handleF = h.handle // Avoid allocating closure during signal
|
||||
noteclear(&h.done)
|
||||
|
||||
defer func() { testSigtrap = nil }()
|
||||
testSigtrap = h.inject
|
||||
if err := tkill(tid); err != nil {
|
||||
return nil, err
|
||||
for i := 0; ; i++ {
|
||||
testSigtrap = h.inject
|
||||
noteclear(&h.done)
|
||||
h.err = ""
|
||||
|
||||
if err := tkill(tid); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Wait for completion.
|
||||
notetsleepg(&h.done, -1)
|
||||
if h.err != "" {
|
||||
switch h.err {
|
||||
case "retry _Grunnable", "executing on Go runtime stack":
|
||||
// These are transient states. Try to get out of them.
|
||||
if i < 100 {
|
||||
Gosched()
|
||||
continue
|
||||
}
|
||||
}
|
||||
return nil, h.err
|
||||
}
|
||||
return h.panic, nil
|
||||
}
|
||||
// Wait for completion.
|
||||
notetsleepg(&h.done, -1)
|
||||
if len(h.err) != 0 {
|
||||
return nil, h.err
|
||||
}
|
||||
return h.panic, nil
|
||||
}
|
||||
|
||||
type debugCallHandler struct {
|
||||
|
@ -99,12 +111,18 @@ func (h *debugCallHandler) inject(info *siginfo, ctxt *sigctxt, gp2 *g) bool {
|
|||
h.savedRegs.fpstate = nil
|
||||
// Set PC to debugCallV1.
|
||||
ctxt.set_rip(uint64(funcPC(debugCallV1)))
|
||||
// Call injected. Switch to the debugCall protocol.
|
||||
testSigtrap = h.handleF
|
||||
case _Grunnable:
|
||||
// Ask InjectDebugCall to pause for a bit and then try
|
||||
// again to interrupt this goroutine.
|
||||
h.err = plainError("retry _Grunnable")
|
||||
notewakeup(&h.done)
|
||||
default:
|
||||
h.err = plainError("goroutine in unexpected state at call inject")
|
||||
return true
|
||||
notewakeup(&h.done)
|
||||
}
|
||||
// Switch to the debugCall protocol and resume execution.
|
||||
testSigtrap = h.handleF
|
||||
// Resume execution.
|
||||
return true
|
||||
}
|
||||
|
||||
|
@ -149,6 +167,7 @@ func (h *debugCallHandler) handle(info *siginfo, ctxt *sigctxt, gp2 *g) bool {
|
|||
sp := ctxt.rsp()
|
||||
reason := *(*string)(unsafe.Pointer(uintptr(sp)))
|
||||
h.err = plainError(reason)
|
||||
// Don't wake h.done. We need to transition to status 16 first.
|
||||
case 16:
|
||||
// Restore all registers except RIP and RSP.
|
||||
rip, rsp := ctxt.rip(), ctxt.rsp()
|
||||
|
@ -162,6 +181,7 @@ func (h *debugCallHandler) handle(info *siginfo, ctxt *sigctxt, gp2 *g) bool {
|
|||
notewakeup(&h.done)
|
||||
default:
|
||||
h.err = plainError("unexpected debugCallV1 status")
|
||||
notewakeup(&h.done)
|
||||
}
|
||||
// Resume execution.
|
||||
return true
|
||||
|
|
Loading…
Reference in a new issue