Skip to content

Commit 3ee7617

Browse files
committed
runtime: free spanQueue on P destroy
Span queues must be empty when destroying a P since we are outside of the mark phase. But we don't actually free them, so they simply sit around using memory. More importantly, they are still in work.spanSPMCs.all, so freeDeadSpanSPMCs must continue traversing past them until the end of time. Prior to CL 709575, keeping them in work.spanSPMCs.all allowed programs with low GOMAXPROCS to continue triggering the bug if they ever had high GOMAXPROCS in the past. The spanSPMCs list is singly-linked, so it is not efficient to remove a random element from the middle. Instead, we simply mark it as dead to all freeDeadSpanSPMCs to free it when it scans the full list. For golang#75771. Change-Id: I6a6a636cfa22a4bdef0c273d083c91553e923fe5 Reviewed-on: https://go-review.googlesource.com/c/go/+/709656 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
1 parent 8709a41 commit 3ee7617

File tree

3 files changed

+39
-0
lines changed

3 files changed

+39
-0
lines changed

src/runtime/mgcmark_greenteagc.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,40 @@ func (q *spanQueue) refill(r *spanSPMC) objptr {
618618
return q.tryGetFast()
619619
}
620620

621+
// destroy frees all chains in an empty spanQueue.
622+
//
623+
// Preconditions:
624+
// - World is stopped.
625+
// - GC is outside of the mark phase.
626+
// - (Therefore) the queue is empty.
627+
func (q *spanQueue) destroy() {
628+
assertWorldStopped()
629+
if gcphase != _GCoff {
630+
throw("spanQueue.destroy during the mark phase")
631+
}
632+
if !q.empty() {
633+
throw("spanQueue.destroy on non-empty queue")
634+
}
635+
636+
lock(&work.spanSPMCs.lock)
637+
638+
// Mark each ring as dead. The sweeper will actually free them.
639+
//
640+
// N.B., we could free directly here, but work.spanSPMCs.all is a
641+
// singly-linked list, so we'd need to walk the entire list to find the
642+
// previous node. If the list becomes doubly-linked, we can free
643+
// directly.
644+
for r := (*spanSPMC)(q.chain.tail.Load()); r != nil; r = (*spanSPMC)(r.prev.Load()) {
645+
r.dead.Store(true)
646+
}
647+
648+
q.chain.head = nil
649+
q.chain.tail.Store(nil)
650+
q.putsSinceDrain = 0
651+
652+
unlock(&work.spanSPMCs.lock)
653+
}
654+
621655
// spanSPMC is a ring buffer of objptrs that represent spans.
622656
// Accessed without a lock.
623657
//

src/runtime/mgcmark_nogreenteagc.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ func (q *spanQueue) empty() bool {
6363
return true
6464
}
6565

66+
func (q *spanQueue) destroy() {
67+
}
68+
6669
type spanSPMC struct {
6770
_ sys.NotInHeap
6871
}

src/runtime/proc.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5824,6 +5824,8 @@ func (pp *p) destroy() {
58245824
println("runtime: p id", pp.id, "destroyed during GC phase", phase)
58255825
throw("P destroyed while GC is running")
58265826
}
5827+
// We should free the queues though.
5828+
pp.gcw.spanq.destroy()
58275829

58285830
clear(pp.sudogbuf[:])
58295831
pp.sudogcache = pp.sudogbuf[:0]

0 commit comments

Comments
 (0)