1berry/go
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

runtime: preempt more aggressively when panicking

Browse Source 
When we are crashing from an unrecovered panic, we freeze the
world, and print stack traces for all goroutines if GOTRACEBACK is
set to a high enough level. Freezing the world is best effort, so
there could still be goroutines that are not preempted, and so its
stack trace is unavailable and printed as "goroutine running on
other thread".

As we're crashing and not resuming execution on preempted
goroutines, we can make preemption more aggressive, preempting
cases that are not safe for resumption or stack scanning. This may
make goroutines more likely to be preempted in freezing the world
and have their stacks available.

Change-Id: Ie16269e2a05e007efa61368b695addc28d7a97ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/546135
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
This commit is contained in:
Cherry Mui 2023-11-29 19:17:22 -05:00
parent b32ec6c961
commit 13766fe7d8
3 changed files with 15 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/runtime/os_windows.go
View File

@ -1298,7 +1298,7 @@ func preemptM(mp *m) {
// Does it want a preemption and is it safe to preempt?
gp := gFromSP(mp, c.sp())
if gp != nil && wantAsyncPreempt(gp) {
if ok, newpc := isAsyncSafePoint(gp, c.ip(), c.sp(), c.lr()); ok {
if ok, newpc := isAsyncSafePoint(gp, c.ip(), c.sp(), c.lr(), panicking.Load() != 0); ok {
// Inject call to asyncPreempt
targetPC := abi.FuncPCABI0(asyncPreempt)
switch GOARCH {

15
src/runtime/preempt.go
View File

@ -359,7 +359,11 @@ func wantAsyncPreempt(gp *g) bool {
// In some cases the PC is safe for asynchronous preemption but it
// also needs to adjust the resumption PC. The new PC is returned in
// the second result.
func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
//
// If noResume is true, we know we're not going to resume execution
// on this goroutine (as we're crashing), and thus we can preempt
// more aggressively.
func isAsyncSafePoint(gp *g, pc, sp, lr uintptr, noResume bool) (bool, uintptr) {
mp := gp.m
// Only user Gs can have safe-points. We check this first
@ -370,7 +374,7 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
}
// Check M state.
if mp.p == 0 || !canPreemptM(mp) {
if mp.p == 0 || (!canPreemptM(mp) && !noResume) {
return false, 0
}
@ -385,6 +389,13 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
// Not Go code.
return false, 0
}
if noResume && f.flag&abi.FuncFlagAsm == 0 {
// We're not going to resume execution and not going to scan the
// stack for GC, so we don't care whether it is a safe point, and
// also don't care the resumption PC.
// TODO: maybe we can preempt non-SPWRITE assembly functions?
return true, pc
}
if (GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "mips64" || GOARCH == "mips64le") && lr == pc+8 && funcspdelta(f, pc) == 0 {
// We probably stopped at a half-executed CALL instruction,
// where the LR is updated but the PC has not. If we preempt

2
src/runtime/signal_unix.go
View File

@ -342,7 +342,7 @@ func doSigPreempt(gp *g, ctxt *sigctxt) {
// Check if this G wants to be preempted and is safe to
// preempt.
if wantAsyncPreempt(gp) {
if ok, newpc := isAsyncSafePoint(gp, ctxt.sigpc(), ctxt.sigsp(), ctxt.siglr()); ok {
if ok, newpc := isAsyncSafePoint(gp, ctxt.sigpc(), ctxt.sigsp(), ctxt.siglr(), panicking.Load() != 0); ok {
// Adjust the PC and inject a call to asyncPreempt.
ctxt.pushCall(abi.FuncPCABI0(asyncPreempt), newpc)
}