From 644493f1090e965cbde3e3245bc8b12bb5486477 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Thu, 14 Apr 2016 08:48:36 -0700 Subject: [PATCH] cmd/compile: clear hidden value at end of channel range body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While we’re here, clean up a few comments. Fixes #15281 Change-Id: Ia6173e9941133db08f57bc80bdd3c5722122bfdb Reviewed-on: https://go-review.googlesource.com/22082 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: David Chase --- src/cmd/compile/internal/gc/range.go | 14 +++--- test/fixedbugs/issue15281.go | 64 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 test/fixedbugs/issue15281.go diff --git a/src/cmd/compile/internal/gc/range.go b/src/cmd/compile/internal/gc/range.go index 6adf8e0d6d..96d7a82972 100644 --- a/src/cmd/compile/internal/gc/range.go +++ b/src/cmd/compile/internal/gc/range.go @@ -154,7 +154,7 @@ func walkrange(n *Node) { v2 = n.List.Second() } - // n->list has no meaning anymore, clear it + // n.List has no meaning anymore, clear it // to avoid erroneous processing by racewalk. n.List.Set(nil) @@ -217,9 +217,9 @@ func walkrange(n *Node) { n.Right.Ninit.Set1(a) } - // orderstmt allocated the iterator for us. - // we only use a once, so no copy needed. case TMAP: + // orderstmt allocated the iterator for us. + // we only use a once, so no copy needed. ha := a th := hiter(t) @@ -254,8 +254,8 @@ func walkrange(n *Node) { body = []*Node{a} } - // orderstmt arranged for a copy of the channel variable. case TCHAN: + // orderstmt arranged for a copy of the channel variable. ha := a n.Left = nil @@ -278,9 +278,13 @@ func walkrange(n *Node) { } else { body = []*Node{Nod(OAS, v1, hv1)} } + // Zero hv1. This prevents hv1 from being the sole, inaccessible + // reference to an otherwise GC-able value during the next channel receive. + // See issue 15281. + body = append(body, Nod(OAS, hv1, nil)) - // orderstmt arranged for a copy of the string variable. case TSTRING: + // orderstmt arranged for a copy of the string variable. ha := a ohv1 := temp(Types[TINT]) diff --git a/test/fixedbugs/issue15281.go b/test/fixedbugs/issue15281.go new file mode 100644 index 0000000000..187c96f218 --- /dev/null +++ b/test/fixedbugs/issue15281.go @@ -0,0 +1,64 @@ +// run + +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. +package main + +import "runtime" + +func main() { + { + x := inuse() + c := make(chan []byte, 10) + c <- make([]byte, 10<<20) + close(c) + f1(c, x) + } + { + x := inuse() + c := make(chan []byte, 10) + c <- make([]byte, 10<<20) + close(c) + f2(c, x) + } +} + +func f1(c chan []byte, start int64) { + for x := range c { + if delta := inuse() - start; delta < 9<<20 { + println("BUG: f1: after alloc: expected delta at least 9MB, got: ", delta) + println(x) + } + x = nil + if delta := inuse() - start; delta > 1<<20 { + println("BUG: f1: after alloc: expected delta below 1MB, got: ", delta) + println(x) + } + } +} + +func f2(c chan []byte, start int64) { + for { + x, ok := <-c + if !ok { + break + } + if delta := inuse() - start; delta < 9<<20 { + println("BUG: f2: after alloc: expected delta at least 9MB, got: ", delta) + println(x) + } + x = nil + if delta := inuse() - start; delta > 1<<20 { + println("BUG: f2: after alloc: expected delta below 1MB, got: ", delta) + println(x) + } + } +} + +func inuse() int64 { + runtime.GC() + var st runtime.MemStats + runtime.ReadMemStats(&st) + return int64(st.Alloc) +}