runtime: clear stale values from G.param and SudoG.elem

This change was necessary on the dev.garbage branch
to keep the garbage collector from seeing pointers into
invalid heap areas.

On this default (Go 1.4) branch, the change removes
some possibility for memory leaks.

LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, r, rlh
https://golang.org/cl/155760043

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 4bb661b54b8c0b2c232cde9e370f6b5ee47a4cd1..76e3ff88516fb25f77961c69770fa90118ec5299 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
        }
@@ -168,6 +171,13 @@ func acquireSudog() *sudog {
 
 //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 504462de330f4114031285dccae210f118c5bcbd..a42a29988a380874317d8e92bbcabddbb171fe6c 100644 (file)
--- a/src/runtime/sema.go
+++ b/src/runtime/sema.go
@@ -173,6 +173,7 @@ func (root *semaRoot) dequeue(s *sudog) {
        } else {
                root.head = s.next
        }
+       s.elem = nil
        s.next = nil
        s.prev = nil
 }