From a3630c9e4419a550af224541e55a0ca6be6f551c Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 2 Oct 2014 16:49:11 -0400 Subject: [PATCH] [dev.garbage] runtime: make sure G.param and SudoG.elem do not hold stale pointers In old conservative Go, this could cause memory leaks. A new pickier collector might reasonably crash when it saw one of these. LGTM=rlh R=rlh CC=golang-codereviews https://golang.org/cl/147480043 --- src/runtime/chan.go | 11 +++++++++-- src/runtime/proc.go | 13 +++++++++++++ src/runtime/select.go | 7 +++++++ src/runtime/sema.go | 1 + 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/runtime/chan.go b/src/runtime/chan.go index 48925b2e3e..10503f4e10 100644 --- a/src/runtime/chan.go +++ b/src/runtime/chan.go @@ -140,10 +140,11 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin unlock(&c.lock) recvg := sg.g - recvg.param = unsafe.Pointer(sg) if sg.elem != nil { memmove(unsafe.Pointer(sg.elem), ep, uintptr(c.elemsize)) + sg.elem = nil } + recvg.param = unsafe.Pointer(sg) if sg.releasetime != 0 { sg.releasetime = cputicks() } @@ -179,6 +180,7 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin } panic("send on closed channel") } + gp.param = nil if mysg.releasetime > 0 { blockevent(int64(mysg.releasetime)-t0, 2) } @@ -278,6 +280,7 @@ func closechan(c *hchan) { break } gp := sg.g + sg.elem = nil gp.param = nil if sg.releasetime != 0 { sg.releasetime = cputicks() @@ -292,6 +295,7 @@ func closechan(c *hchan) { break } gp := sg.g + sg.elem = nil gp.param = nil if sg.releasetime != 0 { sg.releasetime = cputicks() @@ -372,6 +376,7 @@ func chanrecv(t *chantype, c *hchan, ep unsafe.Pointer, block bool) (selected, r if ep != nil { memmove(ep, sg.elem, uintptr(c.elemsize)) } + sg.elem = nil gp := sg.g gp.param = unsafe.Pointer(sg) if sg.releasetime != 0 { @@ -409,9 +414,11 @@ func chanrecv(t *chantype, c *hchan, ep unsafe.Pointer, block bool) (selected, r if mysg.releasetime > 0 { blockevent(mysg.releasetime-t0, 2) } + haveData := gp.param != nil + gp.param = nil releaseSudog(mysg) - if gp.param != nil { + if haveData { // a sender sent us some data. It already wrote to ep. selected = true received = true diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 9b95868594..eefe8239fa 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -148,6 +148,9 @@ func acquireSudog() *sudog { c := gomcache() s := c.sudogcache if s != nil { + if s.elem != nil { + gothrow("acquireSudog: found s.elem != nil in cache") + } c.sudogcache = s.next return s } @@ -162,12 +165,22 @@ func acquireSudog() *sudog { // which keeps the garbage collector from being invoked. mp := acquirem() p := new(sudog) + if p.elem != nil { + gothrow("acquireSudog: found p.elem != nil after new") + } releasem(mp) return p } //go:nosplit func releaseSudog(s *sudog) { + if s.elem != nil { + gothrow("runtime: sudog with non-nil elem") + } + gp := getg() + if gp.param != nil { + gothrow("runtime: releaseSudog with non-nil gp.param") + } c := gomcache() s.next = c.sudogcache c.sudogcache = s diff --git a/src/runtime/select.go b/src/runtime/select.go index 7716d2d4b2..1bcea8c4b4 100644 --- a/src/runtime/select.go +++ b/src/runtime/select.go @@ -368,6 +368,7 @@ loop: // someone woke us up sellock(sel) sg = (*sudog)(gp.param) + gp.param = nil // pass 3 - dequeue from unsuccessful chans // otherwise they stack up on quiet channels @@ -376,6 +377,10 @@ loop: // iterating through the linked list they are in reverse order. cas = nil sglist = gp.waiting + // Clear all elem before unlinking from gp.waiting. + for sg1 := gp.waiting; sg1 != nil; sg1 = sg1.waitlink { + sg1.elem = nil + } gp.waiting = nil for i := int(sel.ncase) - 1; i >= 0; i-- { k = &scases[pollorder[i]] @@ -506,6 +511,7 @@ syncrecv: if cas.elem != nil { memmove(cas.elem, sg.elem, uintptr(c.elemsize)) } + sg.elem = nil gp = sg.g gp.param = unsafe.Pointer(sg) if sg.releasetime != 0 { @@ -541,6 +547,7 @@ syncsend: if sg.elem != nil { memmove(sg.elem, cas.elem, uintptr(c.elemsize)) } + sg.elem = nil gp = sg.g gp.param = unsafe.Pointer(sg) if sg.releasetime != 0 { diff --git a/src/runtime/sema.go b/src/runtime/sema.go index beacd67162..142d3082c0 100644 --- a/src/runtime/sema.go +++ b/src/runtime/sema.go @@ -168,6 +168,7 @@ func (root *semaRoot) dequeue(s *sudog) { } else { root.head = s.next } + s.elem = nil s.next = nil s.prev = nil } -- 2.48.1