[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

diff --git a/src/runtime/chan.go b/src/runtime/chan.go
index 48925b2e3e7595eeab4b4fde3135cc22906b3711..10503f4e105de2aaca84aeafe8686322f35901ba 100644 (file)
--- 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 9b958685949ee2b977545bd9fe615c987c74bce5..eefe8239fad38b8ff7256a9fcee5513b29540e03 100644 (file)
--- 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 7716d2d4b24539af9ecb7ae1833c9cdf45f986a3..1bcea8c4b457a3e46558eeaee48a920e55b8be02 100644 (file)
--- 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 beacd671621b066b053609707abe961943a2b79e..142d3082c0d636f4d47ba15d37d86a2d6074846f 100644 (file)
--- 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
 }