fix: avoid memory leak in closure

Simplify frame management. Remove node dependency on frame pointer.

Fixes #1618

Author: mvertes

_test/issue-1618.go

@@ -0,0 +1,51 @@
+package main
+
+import (
+	"fmt"
+	"runtime"
+	"sync"
+)
+
+func humanizeBytes(bytes uint64) string {
+	const (
+		_         = iota
+		kB uint64 = 1 << (10 * iota)
+		mB
+		gB
+		tB
+		pB
+	)
+
+	switch {
+	case bytes < kB:
+		return fmt.Sprintf("%dB", bytes)
+	case bytes < mB:
+		return fmt.Sprintf("%.2fKB", float64(bytes)/float64(kB))
+	case bytes < gB:
+		return fmt.Sprintf("%.2fMB", float64(bytes)/float64(mB))
+	case bytes < tB:
+		return fmt.Sprintf("%.2fGB", float64(bytes)/float64(gB))
+	case bytes < pB:
+		return fmt.Sprintf("%.2fTB", float64(bytes)/float64(tB))
+	default:
+		return fmt.Sprintf("%dB", bytes)
+	}
+}
+
+func main() {
+	i := 0
+	wg := sync.WaitGroup{}
+
+	for {
+		var m runtime.MemStats
+		runtime.ReadMemStats(&m)
+		fmt.Printf("#%d: alloc = %s, routines = %d, gc = %d\n", i, humanizeBytes(m.Alloc), runtime.NumGoroutine(), m.NumGC)
+
+		wg.Add(1)
+		go func() {
+			wg.Done()
+		}()
+		wg.Wait()
+		i = i + 1
+	}
+}

interp/debugger.go

@@ -272,10 +272,6 @@ func (dbg *Debugger) enterCall(nFunc, nCall *node, f *frame) {
 	switch nFunc.kind {
 	case funcLit:
 		f.debug.kind = frameCall
-		if nFunc.frame != nil {
-			nFunc.frame.debug.kind = frameClosure
-			nFunc.frame.debug.node = nFunc
-		}
 
 	case funcDecl:
 		f.debug.kind = frameCall

interp/interp.go

@@ -33,7 +33,6 @@ type node struct {
 	tnext      *node          // true branch successor (CFG)
 	fnext      *node          // false branch successor (CFG)
 	interp     *Interpreter   // interpreter context
-	frame      *frame         // frame pointer used for closures only (TODO: suppress this)
 	index      int64          // node index (dot display)
 	findex     int            // index of value in frame or frame size (func def, type def)
 	level      int            // number of frame indirections to access value
@@ -138,7 +137,7 @@ func newFrame(anc *frame, length int, id uint64) *frame {
 
 func (f *frame) runid() uint64      { return atomic.LoadUint64(&f.id) }
 func (f *frame) setrunid(id uint64) { atomic.StoreUint64(&f.id, id) }
-func (f *frame) clone(fork bool) *frame {
+func (f *frame) clone() *frame {
 	f.mutex.RLock()
 	defer f.mutex.RUnlock()
 	nf := &frame{
@@ -150,12 +149,8 @@ func (f *frame) clone(fork bool) *frame {
 		done:      f.done,
 		debug:     f.debug,
 	}
-	if fork {
-		nf.data = make([]reflect.Value, len(f.data))
-		copy(nf.data, f.data)
-	} else {
-		nf.data = f.data
-	}
+	nf.data = make([]reflect.Value, len(f.data))
+	copy(nf.data, f.data)
 	return nf
 }
 

interp/interp_consistent_test.go

@@ -80,6 +80,7 @@ func TestInterpConsistencyBuild(t *testing.T) {
 			file.Name() == "time0.go" || // display time (similar to random number)
 			file.Name() == "factor.go" || // bench
 			file.Name() == "fib.go" || // bench
+			file.Name() == "issue-1618.go" || // bench (infinite running)
 
 			file.Name() == "type5.go" || // used to illustrate a limitation with no workaround, related to the fact that the reflect package does not allow the creation of named types
 			file.Name() == "type6.go" || // used to illustrate a limitation with no workaround, related to the fact that the reflect package does not allow the creation of named types

interp/run.go

@@ -988,9 +988,6 @@ func genFunctionWrapper(n *node) func(*frame) reflect.Value {
 	funcType := n.typ.TypeOf()
 
 	return func(f *frame) reflect.Value {
-		if n.frame != nil { // Use closure context if defined.
-			f = n.frame
-		}
 		return reflect.MakeFunc(funcType, func(in []reflect.Value) []reflect.Value {
 			// Allocate and init local frame. All values to be settable and addressable.
 			fr := newFrame(f, len(def.types), f.runid())
@@ -1292,14 +1289,13 @@ func call(n *node) {
 	}
 
 	n.exec = func(f *frame) bltn {
-		var def *node
-		var ok bool
-
+		f.mutex.Lock()
 		bf := value(f)
-
-		if def, ok = bf.Interface().(*node); ok {
+		def, ok := bf.Interface().(*node)
+		if ok {
 			bf = def.rval
 		}
+		f.mutex.Unlock()
 
 		// Call bin func if defined
 		if bf.IsValid() {
@@ -1343,12 +1339,7 @@ func call(n *node) {
 			return tnext
 		}
 
-		anc := f
-		// Get closure frame context (if any)
-		if def.frame != nil {
-			anc = def.frame
-		}
-		nf := newFrame(anc, len(def.types), anc.runid())
+		nf := newFrame(f, len(def.types), f.runid())
 		var vararg reflect.Value
 
 		// Init return values
@@ -1887,27 +1878,22 @@ func getIndexMap2(n *node) {
 	}
 }
 
-const fork = true // Duplicate frame in frame.clone().
-
 // getFunc compiles a closure function generator for anonymous functions.
 func getFunc(n *node) {
 	i := n.findex
 	l := n.level
 	next := getExec(n.tnext)
+	numRet := len(n.typ.ret)
 
 	n.exec = func(f *frame) bltn {
-		fr := f.clone(fork)
-		nod := *n
-		nod.val = &nod
-		nod.frame = fr
-		def := &nod
-		numRet := len(def.typ.ret)
+		fr := f.clone()
+		o := getFrame(f, l).data[i]
 
-		fct := reflect.MakeFunc(nod.typ.TypeOf(), func(in []reflect.Value) []reflect.Value {
+		fct := reflect.MakeFunc(n.typ.TypeOf(), func(in []reflect.Value) []reflect.Value {
 			// Allocate and init local frame. All values to be settable and addressable.
-			fr2 := newFrame(fr, len(def.types), fr.runid())
+			fr2 := newFrame(fr, len(n.types), fr.runid())
 			d := fr2.data
-			for i, t := range def.types {
+			for i, t := range n.types {
 				d[i] = reflect.New(t).Elem()
 			}
 			d = d[numRet:]
@@ -1918,7 +1904,7 @@ func getFunc(n *node) {
 					// In case of unused arg, there may be not even a frame entry allocated, just skip.
 					break
 				}
-				typ := def.typ.arg[i]
+				typ := n.typ.arg[i]
 				switch {
 				case isEmptyInterface(typ) || typ.TypeOf() == valueInterfaceType:
 					d[i].Set(arg)
@@ -1930,12 +1916,19 @@ func getFunc(n *node) {
 			}
 
 			// Interpreter code execution.
-			runCfg(def.child[3].start, fr2, def, n)
+			runCfg(n.child[3].start, fr2, n, n)
+
+			f.mutex.Lock()
+			getFrame(f, l).data[i] = o
+			f.mutex.Unlock()
 
 			return fr2.data[:numRet]
 		})
 
+		f.mutex.Lock()
 		getFrame(f, l).data[i] = fct
+		f.mutex.Unlock()
+
 		return next
 	}
 }
@@ -1946,11 +1939,9 @@ func getMethod(n *node) {
 	next := getExec(n.tnext)
 
 	n.exec = func(f *frame) bltn {
-		fr := f.clone(!fork)
 		nod := *(n.val.(*node))
 		nod.val = &nod
 		nod.recv = n.recv
-		nod.frame = fr
 		getFrame(f, l).data[i] = genFuncValue(&nod)(f)
 		return next
 	}
@@ -2021,11 +2012,9 @@ func getMethodByName(n *node) {
 			panic(n.cfgErrorf("method not found: %s", name))
 		}
 
-		fr := f.clone(!fork)
 		nod := *m
 		nod.val = &nod
 		nod.recv = &receiver{nil, val.value, li}
-		nod.frame = fr
 		getFrame(f, l).data[i] = genFuncValue(&nod)(f)
 		return next
 	}