Skip to content

Commit 8e63961

Browse files
committed
runtime: don't spin in checkPut if non-preemptible
Currently it's possible for the runtime to deadlock if checkPut is called in a non-preemptible context. In this case, checkPut may spin, so it won't leave the non-preemptible context, but the thread running gcMarkDone needs to preempt all of the goroutines before it can release the checkPut spin loops. Fix this by returning from checkPut if it's called under any of the conditions that would prevent gcMarkDone from preempting it. In this case, it leaves a note behind that this happened; if the runtime does later detect left-over work it can at least indicate that it was unable to catch it in the act. For #27993. Updates #29385 (may fix it). Change-Id: Ic71c10701229febb4ddf8c104fb10e06d84b122e Reviewed-on: https://go-review.googlesource.com/c/156017 Run-TryBot: Austin Clements <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rick Hudson <[email protected]>
1 parent 5ec5c57 commit 8e63961

File tree

2 files changed

+19
-0
lines changed

2 files changed

+19
-0
lines changed

src/runtime/mgc.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,6 +1519,9 @@ top:
15191519
print(" wbuf2.n=", gcw.wbuf2.nobj)
15201520
}
15211521
print("\n")
1522+
if gcw.pauseGen == gcw.putGen {
1523+
println("runtime: checkPut already failed at this generation")
1524+
}
15221525
throw("throwOnGCWork")
15231526
}
15241527
}

src/runtime/mgcwork.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ type gcWork struct {
9898
// gcWorkPauseGen if debugCachedWork is true.
9999
pauseGen uint32
100100

101+
// putGen is the pauseGen of the last putGen.
102+
putGen uint32
103+
101104
// pauseStack is the stack at which this P was paused if
102105
// debugCachedWork is true.
103106
pauseStack [16]uintptr
@@ -121,10 +124,23 @@ func (w *gcWork) init() {
121124

122125
func (w *gcWork) checkPut(ptr uintptr, ptrs []uintptr) {
123126
if debugCachedWork {
127+
alreadyFailed := w.putGen == w.pauseGen
128+
w.putGen = w.pauseGen
129+
if m := getg().m; m.locks > 0 || m.mallocing != 0 || m.preemptoff != "" || m.p.ptr().status != _Prunning {
130+
// If we were to spin, the runtime may
131+
// deadlock: the condition above prevents
132+
// preemption (see newstack), which could
133+
// prevent gcMarkDone from finishing the
134+
// ragged barrier and releasing the spin.
135+
return
136+
}
124137
for atomic.Load(&gcWorkPauseGen) == w.pauseGen {
125138
}
126139
if throwOnGCWork {
127140
printlock()
141+
if alreadyFailed {
142+
println("runtime: checkPut already failed at this generation")
143+
}
128144
println("runtime: late gcWork put")
129145
if ptr != 0 {
130146
gcDumpObject("ptr", ptr, ^uintptr(0))

0 commit comments

Comments
 (0)