Skip to content

Commit d42b32e

Browse files
committed
runtime: add sched.lock assertions
Functions that require holding sched.lock now have an assertion. A few places with missing locks have been fixed in this CL: Additionally, locking is added around the call to procresize in schedinit. This doesn't technically need a lock since the program is still starting (thus no concurrency) when this is called, but lock held checking doesn't know that. Updates #40677 Change-Id: I198d3cbaa727f7088e4d55ba8fa989cf1ee8f9cf Reviewed-on: https://go-review.googlesource.com/c/go/+/250261 Run-TryBot: Michael Pratt <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Michael Pratt <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent 53c9b95 commit d42b32e

File tree

1 file changed

+49
-13
lines changed

1 file changed

+49
-13
lines changed

src/runtime/proc.go

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ func schedinit() {
591591
parsedebugvars()
592592
gcinit()
593593

594+
lock(&sched.lock)
594595
sched.lastpoll = uint64(nanotime())
595596
procs := ncpu
596597
if n, ok := atoi32(gogetenv("GOMAXPROCS")); ok && n > 0 {
@@ -599,6 +600,7 @@ func schedinit() {
599600
if procresize(procs) != nil {
600601
throw("unknown runnable goroutine during bootstrap")
601602
}
603+
unlock(&sched.lock)
602604

603605
// For cgocheck > 1, we turn on the write barrier at all times
604606
// and check all pointer writes. We can't do this until after
@@ -629,8 +631,10 @@ func dumpgstatus(gp *g) {
629631
print("runtime: g: g=", _g_, ", goid=", _g_.goid, ", g->atomicstatus=", readgstatus(_g_), "\n")
630632
}
631633

634+
// sched.lock must be held.
632635
func checkmcount() {
633-
// sched lock is held
636+
assertLockHeld(&sched.lock)
637+
634638
if mcount() > sched.maxmcount {
635639
print("runtime: program exceeds ", sched.maxmcount, "-thread limit\n")
636640
throw("thread exhaustion")
@@ -642,6 +646,8 @@ func checkmcount() {
642646
//
643647
// sched.lock must be held.
644648
func mReserveID() int64 {
649+
assertLockHeld(&sched.lock)
650+
645651
if sched.mnext+1 < sched.mnext {
646652
throw("runtime: thread ID overflow")
647653
}
@@ -2545,7 +2551,7 @@ func resetspinning() {
25452551
// Otherwise, for each idle P, this adds a G to the global queue
25462552
// and starts an M. Any remaining G's are added to the current P's
25472553
// local run queue.
2548-
// This may temporarily acquire the scheduler lock.
2554+
// This may temporarily acquire sched.lock.
25492555
// Can run concurrently with GC.
25502556
func injectglist(glist *gList) {
25512557
if glist.empty() {
@@ -4242,6 +4248,8 @@ func (pp *p) init(id int32) {
42424248
//
42434249
// sched.lock must be held and the world must be stopped.
42444250
func (pp *p) destroy() {
4251+
assertLockHeld(&sched.lock)
4252+
42454253
// Move all runnable goroutines to the global queue
42464254
for pp.runqhead != pp.runqtail {
42474255
// Pop from tail of local queue
@@ -4333,11 +4341,17 @@ func (pp *p) destroy() {
43334341
pp.status = _Pdead
43344342
}
43354343

4336-
// Change number of processors. The world is stopped, sched is locked.
4337-
// gcworkbufs are not being modified by either the GC or
4338-
// the write barrier code.
4344+
// Change number of processors.
4345+
//
4346+
// sched.lock must be held, and the world must be stopped.
4347+
//
4348+
// gcworkbufs must not be being modified by either the GC or the write barrier
4349+
// code, so the GC must not be running if the number of Ps actually changes.
4350+
//
43394351
// Returns list of Ps with local work, they need to be scheduled by the caller.
43404352
func procresize(nprocs int32) *p {
4353+
assertLockHeld(&sched.lock)
4354+
43414355
old := gomaxprocs
43424356
if old < 0 || nprocs <= 0 {
43434357
throw("procresize: invalid arg")
@@ -4529,6 +4543,8 @@ func incidlelocked(v int32) {
45294543
// The check is based on number of running M's, if 0 -> deadlock.
45304544
// sched.lock must be held.
45314545
func checkdead() {
4546+
assertLockHeld(&sched.lock)
4547+
45324548
// For -buildmode=c-shared or -buildmode=c-archive it's OK if
45334549
// there are no running goroutines. The calling program is
45344550
// assumed to be running.
@@ -5003,29 +5019,37 @@ func schedEnableUser(enable bool) {
50035019

50045020
// schedEnabled reports whether gp should be scheduled. It returns
50055021
// false is scheduling of gp is disabled.
5022+
//
5023+
// sched.lock must be held.
50065024
func schedEnabled(gp *g) bool {
5025+
assertLockHeld(&sched.lock)
5026+
50075027
if sched.disable.user {
50085028
return isSystemGoroutine(gp, true)
50095029
}
50105030
return true
50115031
}
50125032

50135033
// Put mp on midle list.
5014-
// Sched must be locked.
5034+
// sched.lock must be held.
50155035
// May run during STW, so write barriers are not allowed.
50165036
//go:nowritebarrierrec
50175037
func mput(mp *m) {
5038+
assertLockHeld(&sched.lock)
5039+
50185040
mp.schedlink = sched.midle
50195041
sched.midle.set(mp)
50205042
sched.nmidle++
50215043
checkdead()
50225044
}
50235045

50245046
// Try to get an m from midle list.
5025-
// Sched must be locked.
5047+
// sched.lock must be held.
50265048
// May run during STW, so write barriers are not allowed.
50275049
//go:nowritebarrierrec
50285050
func mget() *m {
5051+
assertLockHeld(&sched.lock)
5052+
50295053
mp := sched.midle.ptr()
50305054
if mp != nil {
50315055
sched.midle = mp.schedlink
@@ -5035,35 +5059,43 @@ func mget() *m {
50355059
}
50365060

50375061
// Put gp on the global runnable queue.
5038-
// Sched must be locked.
5062+
// sched.lock must be held.
50395063
// May run during STW, so write barriers are not allowed.
50405064
//go:nowritebarrierrec
50415065
func globrunqput(gp *g) {
5066+
assertLockHeld(&sched.lock)
5067+
50425068
sched.runq.pushBack(gp)
50435069
sched.runqsize++
50445070
}
50455071

50465072
// Put gp at the head of the global runnable queue.
5047-
// Sched must be locked.
5073+
// sched.lock must be held.
50485074
// May run during STW, so write barriers are not allowed.
50495075
//go:nowritebarrierrec
50505076
func globrunqputhead(gp *g) {
5077+
assertLockHeld(&sched.lock)
5078+
50515079
sched.runq.push(gp)
50525080
sched.runqsize++
50535081
}
50545082

50555083
// Put a batch of runnable goroutines on the global runnable queue.
50565084
// This clears *batch.
5057-
// Sched must be locked.
5085+
// sched.lock must be held.
50585086
func globrunqputbatch(batch *gQueue, n int32) {
5087+
assertLockHeld(&sched.lock)
5088+
50595089
sched.runq.pushBackAll(*batch)
50605090
sched.runqsize += n
50615091
*batch = gQueue{}
50625092
}
50635093

50645094
// Try get a batch of G's from the global runnable queue.
5065-
// Sched must be locked.
5095+
// sched.lock must be held.
50665096
func globrunqget(_p_ *p, max int32) *g {
5097+
assertLockHeld(&sched.lock)
5098+
50675099
if sched.runqsize == 0 {
50685100
return nil
50695101
}
@@ -5091,10 +5123,12 @@ func globrunqget(_p_ *p, max int32) *g {
50915123
}
50925124

50935125
// Put p to on _Pidle list.
5094-
// Sched must be locked.
5126+
// sched.lock must be held.
50955127
// May run during STW, so write barriers are not allowed.
50965128
//go:nowritebarrierrec
50975129
func pidleput(_p_ *p) {
5130+
assertLockHeld(&sched.lock)
5131+
50985132
if !runqempty(_p_) {
50995133
throw("pidleput: P has non-empty run queue")
51005134
}
@@ -5104,10 +5138,12 @@ func pidleput(_p_ *p) {
51045138
}
51055139

51065140
// Try get a p from _Pidle list.
5107-
// Sched must be locked.
5141+
// sched.lock must be held.
51085142
// May run during STW, so write barriers are not allowed.
51095143
//go:nowritebarrierrec
51105144
func pidleget() *p {
5145+
assertLockHeld(&sched.lock)
5146+
51115147
_p_ := sched.pidle.ptr()
51125148
if _p_ != nil {
51135149
sched.pidle = _p_.link

0 commit comments

Comments
 (0)