runtime: guarantee no GOMAXPROCS update syscalls after GOMAXPROCS call

We already guarantee that no automatic updates to GOMAXPROCS occur after
a GOMAXPROCS call returns. This is easily achieved by having the update
goroutine double-check that updates are still allowed during STW before
committing the new value.

However, it is possible for sysmon to concurrently run defaultGOMAXPROCS
to compute a new GOMAXPROCS value after GOMAXPROCS returns. This new
value will be discarded later, but we'll still perform the system calls
necessary to compute the new value.

Normally this distinction doesn't matter, but if you want to sandbox a
Go program, then you may want to disable GOMAXPROCS updates to reduce
the system call footprint. A call to GOMAXPROCS will disable updates,
but without a guarantee on when sysmon will observe the change it is
somewhat fragile.

Add explicit synchronization between GOMAXPROCS and sysmon to guarantee
that sysmon won't run defaultGOMAXPROCS after GOMAXPROCS returns.

The synchronization is a bit complex because we can't hold a mutex
across STW, nor take a semaphore from sysmon, but the result isn't too
bad.

One oddity is that sched.customGOMAXPROCS and gomaxprocs are no longer
updated in lockstep (even though both are protected by sched.lock), but
I don't believe anything should depend on that.

For #73193.

Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-staticlockranking
Change-Id: I6a6a636cff243a9b69ac1b5d2f98925648e60236
Reviewed-on: https://go-review.googlesource.com/c/go/+/677037
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

1 files changed, 60 insertions, 3 deletions

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 4ccce90e93..5b8db2bee4 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -835,6 +835,7 @@ func schedinit() {
 	lockInit(&reflectOffs.lock, lockRankReflectOffs)
 	lockInit(&finlock, lockRankFin)
 	lockInit(&cpuprof.lock, lockRankCpuprof)
+	lockInit(&computeMaxProcsLock, lockRankComputeMaxProcs)
 	allocmLock.init(lockRankAllocmR, lockRankAllocmRInternal, lockRankAllocmW)
 	execLock.init(lockRankExecR, lockRankExecRInternal, lockRankExecW)
 	traceLockInit()
@@ -1727,7 +1728,6 @@ func startTheWorldWithSema(now int64, w worldStop) int64 {
 	procs := gomaxprocs
 	if newprocs != 0 {
 		procs = newprocs
-		sched.customGOMAXPROCS = newprocsCustom
 		newprocs = 0
 	}
 	p1 := procresize(procs)
@@ -6560,7 +6560,58 @@ var (
 	// GOMAXPROCS updates actually change the value of GOMAXPROCS.
 	updatemaxprocs = &godebugInc{name: "updatemaxprocs"}
 
+	// Synchronization and state between updateMaxProcsGoroutine and
+	// sysmon.
 	updateMaxProcsG updateMaxProcsGState
+
+	// Synchronization between GOMAXPROCS and sysmon.
+	//
+	// Setting GOMAXPROCS via a call to GOMAXPROCS disables automatic
+	// GOMAXPROCS updates.
+	//
+	// We want to make two guarantees to callers of GOMAXPROCS. After
+	// GOMAXPROCS returns:
+	//
+	// 1. The runtime will not make any automatic changes to GOMAXPROCS.
+	//
+	// 2. The runtime will not perform any of the system calls used to
+	//    determine the appropriate value of GOMAXPROCS (i.e., it won't
+	//    call defaultGOMAXPROCS).
+	//
+	// (1) is the baseline guarantee that everyone needs. The GOMAXPROCS
+	// API isn't useful to anyone if automatic updates may occur after it
+	// returns. This is easily achieved by double-checking the state under
+	// STW before committing an automatic GOMAXPROCS update.
+	//
+	// (2) doesn't matter to most users, as it is isn't observable as long
+	// as (1) holds. However, it can be important to users sandboxing Go.
+	// They want disable these system calls and need some way to know when
+	// they are guaranteed the calls will stop.
+	//
+	// This would be simple to achieve if we simply called
+	// defaultGOMAXPROCS under STW in updateMaxProcsGoroutine below.
+	// However, we would like to avoid scheduling this goroutine every
+	// second when it will almost never do anything. Instead, sysmon calls
+	// defaultGOMAXPROCS to decide whether to schedule
+	// updateMaxProcsGoroutine. Thus we need to synchronize between sysmon
+	// and GOMAXPROCS calls.
+	//
+	// GOMAXPROCS can't hold a runtime mutex across STW. It could hold a
+	// semaphore, but sysmon cannot take semaphores. Instead, we have a
+	// more complex scheme:
+	//
+	// * sysmon holds computeMaxProcsLock while calling defaultGOMAXPROCS.
+	// * sysmon skips the current update if sched.customGOMAXPROCS is
+	//   set.
+	// * GOMAXPROCS sets sched.customGOMAXPROCS once it is committed to
+	//   changing GOMAXPROCS.
+	// * GOMAXPROCS takes computeMaxProcsLock to wait for outstanding
+	//   defaultGOMAXPROCS calls to complete.
+	//
+	// N.B. computeMaxProcsLock could simply be sched.lock, but we want to
+	// avoid holding that lock during the potentially slow
+	// defaultGOMAXPROCS.
+	computeMaxProcsLock mutex
 )
 
 // Start GOMAXPROCS update helper goroutine.
@@ -6614,25 +6665,31 @@ func updateMaxProcsGoroutine() {
 		// TODO(prattmic): this could use a nicer API. Perhaps add it to the
 		// stw parameter?
 		newprocs = updateMaxProcsG.procs
-		newprocsCustom = false
+		lock(&sched.lock)
+		sched.customGOMAXPROCS = false
+		unlock(&sched.lock)
 
 		startTheWorldGC(stw)
 	}
 }
 
 func sysmonUpdateGOMAXPROCS() {
+	// Synchronize with GOMAXPROCS. See comment on computeMaxProcsLock.
+	lock(&computeMaxProcsLock)
+
 	// No update if GOMAXPROCS was set manually.
 	lock(&sched.lock)
 	custom := sched.customGOMAXPROCS
 	curr := gomaxprocs
 	unlock(&sched.lock)
 	if custom {
+		unlock(&computeMaxProcsLock)
 		return
 	}
 
 	// Don't hold sched.lock while we read the filesystem.
 	procs := defaultGOMAXPROCS(0)
-
+	unlock(&computeMaxProcsLock)
 	if procs == curr {
 		// Nothing to do.
 		return