Revert "runtime: preempt more aggressively when panicking"

This reverts CL 546135.

Reason for revert: Causes occasional throw during panic

For #65416.

Change-Id: I78c15637da18f85ede785363b777aa7d1dead3c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/560455
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 50f61c1d54162860ba4dcda056df41a3f8c8b0a7..7e9bbd04f2758807ac59a4e9537ea4654cab1a5c 100644 (file)
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -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(), panicking.Load() != 0); ok {
+               if ok, newpc := isAsyncSafePoint(gp, c.ip(), c.sp(), c.lr()); ok {
                        // Inject call to asyncPreempt
                        targetPC := abi.FuncPCABI0(asyncPreempt)
                        switch GOARCH {

diff --git a/src/runtime/preempt.go b/src/runtime/preempt.go
index aedf7f3ff59e219666ba21790af7348de395e84b..82d85cd707619a5e140c018ad005f7e9e2c5779f 100644 (file)
--- a/src/runtime/preempt.go
+++ b/src/runtime/preempt.go
@@ -359,11 +359,7 @@ 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.
-//
-// 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) {
+func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
        mp := gp.m
 
        // Only user Gs can have safe-points. We check this first
@@ -374,7 +370,7 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr, noResume bool) (bool, uintptr)
        }
 
        // Check M state.
-       if mp.p == 0 || (!canPreemptM(mp) && !noResume) {
+       if mp.p == 0 || !canPreemptM(mp) {
                return false, 0
        }
 
@@ -389,13 +385,6 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr, noResume bool) (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

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index 78e71c908697c445e2327f01b4db804ecef4a6d3..84391d58ed7f22404ee33ba8ecf9174b513f8890 100644 (file)
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -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(), panicking.Load() != 0); ok {
+               if ok, newpc := isAsyncSafePoint(gp, ctxt.sigpc(), ctxt.sigsp(), ctxt.siglr()); ok {
                        // Adjust the PC and inject a call to asyncPreempt.
                        ctxt.pushCall(abi.FuncPCABI0(asyncPreempt), newpc)
                }