From a189516d3a1623f2560f80569c4b64bdffc4ac78 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 9 Jun 2025 21:45:33 +0000 Subject: [PATCH] runtime: don't do a direct G handoff in semrelease on systemstack semrelease is safe to call on the system stack (since it just readies goroutines) except for the fact that it might perform a direct G handoff and call into the scheduler. If handoff is set to false this is exceptionally rare, but could happen, and has happened for the trace reader goroutine which releases a trace.doneSema. Fixes #73469. Change-Id: I37ece678bc4721bbb6e5879d74daac762b7d742a Reviewed-on: https://go-review.googlesource.com/c/go/+/680315 Auto-Submit: Michael Knyszek LUCI-TryBot-Result: Go LUCI Reviewed-by: Carlos Amedee --- src/runtime/sema.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/runtime/sema.go b/src/runtime/sema.go index 0f029f604f..6af49b1b0c 100644 --- a/src/runtime/sema.go +++ b/src/runtime/sema.go @@ -261,11 +261,13 @@ func semrelease1(addr *uint32, handoff bool, skipframes int) { s.ticket = 1 } readyWithTime(s, 5+skipframes) - if s.ticket == 1 && getg().m.locks == 0 { + if s.ticket == 1 && getg().m.locks == 0 && getg() != getg().m.g0 { // Direct G handoff + // // readyWithTime has added the waiter G as runnext in the // current P; we now call the scheduler so that we start running // the waiter G immediately. + // // Note that waiter inherits our time slice: this is desirable // to avoid having a highly contended semaphore hog the P // indefinitely. goyield is like Gosched, but it emits a @@ -275,9 +277,12 @@ func semrelease1(addr *uint32, handoff bool, skipframes int) { // the non-starving case it is possible for a different waiter // to acquire the semaphore while we are yielding/scheduling, // and this would be wasteful. We wait instead to enter starving - // regime, and then we start to do direct handoffs of ticket and - // P. + // regime, and then we start to do direct handoffs of ticket and P. + // // See issue 33747 for discussion. + // + // We don't handoff directly if we're holding locks or on the + // system stack, since it's not safe to enter the scheduler. goyield() } } -- 2.50.0