runtime: cleanup M vgetrandom state before dropping P

When an M is destroyed, we put its vgetrandom state back on the shared
list for another M to reuse. This list is simply a slice, so appending
to the slice may allocate. Currently this operation is performed in
mdestroy, after the P is released, meaning allocation is not allowed.

More the cleanup earlier in mdestroy when allocation is still OK.

Also add //go:nowritebarrierrec to mdestroy since it runs without a P,
which would have caught this bug.

Fixes #73141.

Change-Id: I6a6a636c3fbf5c6eec09d07a260e39dbb4d2db12
Reviewed-on: https://go-review.googlesource.com/c/go/+/662455
Reviewed-by: Jason Donenfeld <Jason@zx2c4.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>

diff --git a/src/runtime/os3_solaris.go b/src/runtime/os3_solaris.go
index cf163a6bf401f69fafe174febdc056c10bc3c8a4..ded821b2e66f817a19e26c24166153bf42159f77 100644 (file)
--- a/src/runtime/os3_solaris.go
+++ b/src/runtime/os3_solaris.go
@@ -234,8 +234,11 @@ func unminit() {
        getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_aix.go b/src/runtime/os_aix.go
index 93464cb997f3bea818ec9a192153156d5b133e7a..1b483c2a7e98567410a6041e292cfd00a7cc77a3 100644 (file)
--- a/src/runtime/os_aix.go
+++ b/src/runtime/os_aix.go
@@ -186,8 +186,11 @@ func unminit() {
        getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go
index 0ecbea7ae47c609f97f5a924b8e991c80d64fb90..6eab3b5c3ddba63178a712fc798062fd58769baf 100644 (file)
--- a/src/runtime/os_darwin.go
+++ b/src/runtime/os_darwin.go
@@ -344,8 +344,11 @@ func unminit() {
        getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_dragonfly.go b/src/runtime/os_dragonfly.go
index a02696eb4f566dfc25747bd69cd32ac5863ad1b1..9b3235084d7c4fd1264424b9c82466e0aaaac6c5 100644 (file)
--- a/src/runtime/os_dragonfly.go
+++ b/src/runtime/os_dragonfly.go
@@ -216,8 +216,11 @@ func unminit() {
        getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go
index 55c4ac8f61f9c6b3e492352099be775acec52afd..f24d18027be8af2d0f54e5de79ba5da2855905f0 100644 (file)
--- a/src/runtime/os_linux.go
+++ b/src/runtime/os_linux.go
@@ -412,13 +412,12 @@ func unminit() {
        getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
-       if mp.vgetrandomState != 0 {
-               vgetrandomPutState(mp.vgetrandomState)
-               mp.vgetrandomState = 0
-       }
 }
 
 // #ifdef GOARCH_386

diff --git a/src/runtime/os_netbsd.go b/src/runtime/os_netbsd.go
index 735ace25adb3b331e6202d7b1e6a8d9ee98e29c7..a06e5febbd5a75a200abe1642555526c3b429ea1 100644 (file)
--- a/src/runtime/os_netbsd.go
+++ b/src/runtime/os_netbsd.go
@@ -320,8 +320,11 @@ func unminit() {
        // must continue working after unminit.
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_openbsd.go b/src/runtime/os_openbsd.go
index 574bfa8b1708be7861d3f605ce1d4c58356338ef..4ce4c3c58d35df6d984ca6c583c8e5c111aea063 100644 (file)
--- a/src/runtime/os_openbsd.go
+++ b/src/runtime/os_openbsd.go
@@ -182,8 +182,11 @@ func unminit() {
        getg().m.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_plan9.go b/src/runtime/os_plan9.go
index b45e409b3a4059e28886d59b2c88d97f1f408ab2..6ddf1d21d4a385e7c5bedd2ab6ba09514014067c 100644 (file)
--- a/src/runtime/os_plan9.go
+++ b/src/runtime/os_plan9.go
@@ -218,8 +218,11 @@ func minit() {
 func unminit() {
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
+//
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 func mdestroy(mp *m) {
 }
 

diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 489b396fc9c45a0c9a1fd1283d9e5edf995e6bd0..a84832e0ce35556c56fef6314bb142d678a3f1da 100644 (file)
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -905,9 +905,11 @@ func unminit() {
        mp.procid = 0
 }
 
-// Called from exitm, but not from drop, to undo the effect of thread-owned
+// Called from mexit, but not from dropm, to undo the effect of thread-owned
 // resources in minit, semacreate, or elsewhere. Do not take locks after calling this.
 //
+// This always runs without a P, so //go:nowritebarrierrec is required.
+//go:nowritebarrierrec
 //go:nosplit
 func mdestroy(mp *m) {
        if mp.highResTimer != 0 {

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 74c19e9e435bf8cf4666656f4718fd8486f46769..cee91b6ce85d57de4cf8e8a27f39bd494a495a5c 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -1958,6 +1958,9 @@ func mexit(osStack bool) {
                mp.gsignal = nil
        }
 
+       // Free vgetrandom state.
+       vgetrandomDestroy(mp)
+
        // Remove m from allm.
        lock(&sched.lock)
        for pprev := &allm; *pprev != nil; pprev = &(*pprev).alllink {

diff --git a/src/runtime/vgetrandom_linux.go b/src/runtime/vgetrandom_linux.go
index 00ef2c2d4ee623de83cbca98a77a208d2aca5e2b..6ab12e3c67b07a300aa2930556231a8c3475c5c1 100644 (file)
--- a/src/runtime/vgetrandom_linux.go
+++ b/src/runtime/vgetrandom_linux.go
@@ -74,9 +74,16 @@ func vgetrandomGetState() uintptr {
        return state
 }
 
-func vgetrandomPutState(state uintptr) {
+// Free vgetrandom state from the M (if any) prior to destroying the M.
+//
+// This may allocate, so it must have a P.
+func vgetrandomDestroy(mp *m) {
+       if mp.vgetrandomState == 0 {
+               return
+       }
+
        lock(&vgetrandomAlloc.statesLock)
-       vgetrandomAlloc.states = append(vgetrandomAlloc.states, state)
+       vgetrandomAlloc.states = append(vgetrandomAlloc.states, mp.vgetrandomState)
        unlock(&vgetrandomAlloc.statesLock)
 }
 

diff --git a/src/runtime/vgetrandom_unsupported.go b/src/runtime/vgetrandom_unsupported.go
index 070392cfaaf7a901bcaf9db9ef014e8d8e5ce0a4..43c53e1198843c4f8af85a9246c476ff698b4da4 100644 (file)
--- a/src/runtime/vgetrandom_unsupported.go
+++ b/src/runtime/vgetrandom_unsupported.go
@@ -13,6 +13,6 @@ func vgetrandom(p []byte, flags uint32) (ret int, supported bool) {
        return -1, false
 }
 
-func vgetrandomPutState(state uintptr) {}
+func vgetrandomDestroy(mp *m) {}
 
 func vgetrandomInit() {}