]> Cypherpunks repositories - gostls13.git/commit
runtime: hold sched.lock across atomic pidleget/pidleput
authorMichael Pratt <mpratt@google.com>
Mon, 10 May 2021 20:50:32 +0000 (16:50 -0400)
committerMichael Pratt <mpratt@google.com>
Tue, 11 May 2021 14:21:06 +0000 (14:21 +0000)
commit2520e72d3bbfe6651cd6324077afdb4babb36b9a
treec379ba5d793d3cfe83892a418fe9e085881c1aee
parent326a7925179ea669aa9f947dda82e425673cb220
runtime: hold sched.lock across atomic pidleget/pidleput

As a cleanup, golang.org/cl/307914 unintentionally caused the idle GC
work recheck to drop sched.lock between acquiring a P and committing to
keep it (once a worker G was found).

This is unsafe, as releasing a P requires extra checks once sched.lock
is taken (such as for runSafePointFn). Since checkIdleGCNoP does not
perform these extra checks, we can now race with other users.

In the case of #45975, we may hang with this sequence:

1. M1: checkIdleGCNoP takes sched.lock, gets P1, releases sched.lock.
2. M2: forEachP takes sched.lock, iterates over sched.pidle without
   finding P1, releases sched.lock.
3. M1: checkIdleGCNoP puts P1 back in sched.pidle.
4. M2: forEachP waits forever for P1 to run the safePointFn.

Change back to the old behavior of releasing sched.lock only after we
are certain we will keep the P. Thus if we put it back its removal from
sched.pidle was never visible.

Fixes #45975
For #45916
For #45885
For #45884

Change-Id: I191a1800923b206ccaf96bdcdd0bfdad17b532e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/318569
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
src/runtime/proc.go