go/ssa/interp: fix goroutines and channels leaks
rangeIter leaks goroutines and channels
on each incomplete map iteration.
So remove goroutines and channels iterators
and use reflect.(Value).MapRange() iteration instead.
Resolves TODO left by @adonovan
Change-Id: I8cc24b264ae782376c392fd9497be81bf50a415f
GitHub-Last-Rev: 5bba70b8658575b3e2e33b8941fce3ef75199a4f
GitHub-Pull-Request: golang/tools#198
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215457
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/go/ssa/interp/interp.go b/go/ssa/interp/interp.go
index 76b7ca3..d776594 100644
--- a/go/ssa/interp/interp.go
+++ b/go/ssa/interp/interp.go
@@ -32,8 +32,6 @@
// makes no attempt to distinguish target panics from interpreter
// crashes.
//
-// * map iteration is asymptotically inefficient.
-//
// * the sizes of the int, uint and uintptr types in the target
// program are assumed to be the same as those of the interpreter
// itself.
diff --git a/go/ssa/interp/ops.go b/go/ssa/interp/ops.go
index ca004d7..fdc0161 100644
--- a/go/ssa/interp/ops.go
+++ b/go/ssa/interp/ops.go
@@ -11,6 +11,7 @@
"go/token"
"go/types"
"os"
+ "reflect"
"strings"
"sync"
"unsafe"
@@ -1077,34 +1078,9 @@
func rangeIter(x value, t types.Type) iter {
switch x := x.(type) {
case map[value]value:
- // TODO(adonovan): fix: leaks goroutines and channels
- // on each incomplete map iteration. We need to open
- // up an iteration interface using the
- // reflect.(Value).MapKeys machinery.
- it := make(mapIter)
- go func() {
- for k, v := range x {
- it <- [2]value{k, v}
- }
- close(it)
- }()
- return it
+ return &mapIter{iter: reflect.ValueOf(x).MapRange()}
case *hashmap:
- // TODO(adonovan): fix: leaks goroutines and channels
- // on each incomplete map iteration. We need to open
- // up an iteration interface using the
- // reflect.(Value).MapKeys machinery.
- it := make(mapIter)
- go func() {
- for _, e := range x.entries() {
- for e != nil {
- it <- [2]value{e.key, e.value}
- e = e.next
- }
- }
- close(it)
- }()
- return it
+ return &hashmapIter{iter: reflect.ValueOf(x.entries()).MapRange()}
case string:
return &stringIter{Reader: strings.NewReader(x)}
}
diff --git a/go/ssa/interp/value.go b/go/ssa/interp/value.go
index 4faa7b8..94018b5 100644
--- a/go/ssa/interp/value.go
+++ b/go/ssa/interp/value.go
@@ -489,9 +489,37 @@
return okv
}
-type mapIter chan [2]value
+type mapIter struct {
+ iter *reflect.MapIter
+ ok bool
+}
-func (it mapIter) next() tuple {
- kv, ok := <-it
- return tuple{ok, kv[0], kv[1]}
+func (it *mapIter) next() tuple {
+ it.ok = it.iter.Next()
+ if !it.ok {
+ return []value{false, nil, nil}
+ }
+ k, v := it.iter.Key().Interface(), it.iter.Value().Interface()
+ return []value{true, k, v}
+}
+
+type hashmapIter struct {
+ iter *reflect.MapIter
+ ok bool
+ cur *entry
+}
+
+func (it *hashmapIter) next() tuple {
+ for {
+ if it.cur != nil {
+ k, v := it.cur.key, it.cur.value
+ it.cur = it.cur.next
+ return []value{true, k, v}
+ }
+ it.ok = it.iter.Next()
+ if !it.ok {
+ return []value{false, nil, nil}
+ }
+ it.cur = it.iter.Value().Interface().(*entry)
+ }
}
