fix: avoid memory leak in closure

Simplify frame management. Remove node dependency on frame pointer.

Fixes #1618

.github/workflows/go-cross.yml

@@ -17,7 +17,7 @@ jobs:
 
     strategy:
       matrix:
-        go-version: [ '1.21', '1.22' ]
+        go-version: [ oldstable, stable ]
         os: [ubuntu-latest, macos-latest, windows-latest]
 
         include:
@@ -29,36 +29,18 @@ jobs:
             go-path-suffix: \go
 
     steps:
-      # https://github.com/marketplace/actions/setup-go-environment
-      - name: Set up Go ${{ matrix.go-version }}
-        uses: actions/setup-go@v2
-        with:
-          go-version: ${{ matrix.go-version }}
-          stable: true
-
       # https://github.com/marketplace/actions/checkout
       - name: Checkout code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
         with:
           path: go/src/github.com/traefik/yaegi
 
-      # https://github.com/marketplace/actions/cache
-      - name: Cache Go modules
-        uses: actions/cache@v3
+      # https://github.com/marketplace/actions/setup-go-environment
+      - name: Set up Go ${{ matrix.go-version }}
+        uses: actions/setup-go@v5
         with:
-          # In order:
-          # * Module download cache
-          # * Build cache (Linux)
-          # * Build cache (Mac)
-          # * Build cache (Windows)
-          path: |
-            ~/go/pkg/mod
-            ~/.cache/go-build
-            ~/Library/Caches/go-build
-            %LocalAppData%\go-build
-          key: ${{ runner.os }}-${{ matrix.go-version }}-go-${{ hashFiles('**/go.sum') }}
-          restore-keys: |
-            ${{ runner.os }}-${{ matrix.go-version }}-go-
+          go-version: ${{ matrix.go-version }}
+          stable: true
 
       - name: Setup GOPATH
         run: go env -w GOPATH=${{ github.workspace }}${{ matrix.go-path-suffix }}

.github/workflows/main.yml

@@ -7,8 +7,8 @@ on:
   pull_request:
 
 env:
-  GO_VERSION: '1.22'
-  GOLANGCI_LINT_VERSION: v1.55.2
+  GO_VERSION: stable
+  GOLANGCI_LINT_VERSION: v1.56.2
 
 jobs:
 
@@ -16,16 +16,16 @@ jobs:
     name: Linting
     runs-on: ubuntu-latest
     steps:
-      - name: Set up Go ${{ env.GO_VERSION }}
-        uses: actions/setup-go@v2
-        with:
-          go-version: ${{ env.GO_VERSION }}
-
       - name: Check out code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
+      - name: Set up Go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+
       - name: Check and get dependencies
         run: |
           go mod tidy
@@ -45,19 +45,19 @@ jobs:
     needs: linting
     strategy:
       matrix:
-        go-version: [ '1.21', '1.22' ]
+        go-version: [ oldstable, stable ]
     steps:
+      - name: Check out code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
       - name: Set up Go ${{ matrix.go-version }}
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
           go-version: ${{ matrix.go-version }}
           stable: true
 
-      - name: Check out code
-        uses: actions/checkout@v2
-        with:
-          fetch-depth: 0
-
       - name: Check generated code
         run: |
           rm -f interp/op.go
@@ -76,21 +76,21 @@ jobs:
         working-directory: ${{ github.workspace }}/go/src/github.com/traefik/yaegi
     strategy:
       matrix:
-        go-version: [ '1.21', '1.22' ]
+        go-version: [ oldstable, stable ]
 
     steps:
-      - name: Set up Go ${{ matrix.go-version }}
-        uses: actions/setup-go@v2
-        with:
-          go-version: ${{ matrix.go-version }}
-          stable: true
-
       - name: Check out code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
         with:
           path: go/src/github.com/traefik/yaegi
           fetch-depth: 0
 
+      - name: Set up Go ${{ matrix.go-version }}
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ matrix.go-version }}
+          stable: true
+
       # https://github.com/marketplace/actions/cache
       - name: Cache Go modules
         uses: actions/cache@v3

.github/workflows/release.yml

@@ -6,7 +6,7 @@ on:
       - v[0-9]+.[0-9]+*
 
 env:
-  GO_VERSION: '1.21'
+  GO_VERSION: stable
 
 jobs:
 
@@ -15,28 +15,20 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - name: Set up Go ${{ env.GO_VERSION }}
-        uses: actions/setup-go@v2
-        with:
-          go-version: ${{ env.GO_VERSION }}
-
       - name: Check out code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
-      - name: Cache Go modules
-        uses: actions/cache@v3
+      - name: Set up Go ${{ env.GO_VERSION }}
+        uses: actions/setup-go@v5
         with:
-          path: ~/go/pkg/mod
-          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
-          restore-keys: |
-            ${{ runner.os }}-go-
+          go-version: ${{ env.GO_VERSION }}
 
       - name: Run GoReleaser
-        uses: goreleaser/goreleaser-action@v2
+        uses: goreleaser/goreleaser-action@v5
         with:
           version: latest
-          args: release --rm-dist
+          args: release --clean
         env:
           GITHUB_TOKEN: ${{ secrets.GH_TOKEN_REPO }}

.gitignore

@@ -1,6 +1,7 @@
 .*.swo
 .*.swp
 *.dot
+*.out
 .idea/
 /yaegi
 internal/cmd/extract/extract

.golangci.yml

@@ -142,9 +142,10 @@ linters:
 
 issues:
   exclude-use-default: false
-  max-per-linter: 0
+  max-issues-per-linter: 0
   max-same-issues: 0
-  exclude: []
+  exclude:
+    - 'fmt.Sprintf can be replaced with string'
   exclude-rules:
     - path: .+_test\.go
       linters:

.goreleaser.yml

@@ -47,7 +47,7 @@ archives:
       - LICENSE
 
 brews:
-  - tap:
+  - repository:
       owner: traefik
       name: homebrew-tap
     commit_author:

_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
+	}
+}

_test/op10.go

@@ -0,0 +1,9 @@
+package main
+
+func main() {
+	_ = 1 + 1
+	println("ok")
+}
+
+// Output:
+// ok

_test/op11.go

@@ -0,0 +1,10 @@
+package main
+
+func main() {
+	a, b := 1, 2
+	_ = a + b
+	println("ok")
+}
+
+// Output:
+// ok

cmd/yaegi/extract.go

@@ -3,6 +3,7 @@ package main
 import (
 	"bufio"
 	"bytes"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -38,7 +39,7 @@ func extractCmd(arg []string) error {
 
 	args := eflag.Args()
 	if len(args) == 0 {
-		return fmt.Errorf("missing package")
+		return errors.New("missing package")
 	}
 
 	license, err := genLicense(licensePath)

extract/extract.go

@@ -387,7 +387,7 @@ func (e *Extractor) importPath(pkgIdent, importPath string) (string, error) {
 		return "", err
 	}
 	if err != nil {
-		if len(pkgIdent) > 0 && pkgIdent[0] == '.' {
+		if pkgIdent != "" && pkgIdent[0] == '.' {
 			// pkgIdent is definitely a relative path, not a package name, and it does not exist
 			return "", err
 		}

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
 	}

interp/type.go

@@ -2393,7 +2393,7 @@ func isMap(t *itype) bool  { return t.TypeOf().Kind() == reflect.Map }
 func isPtr(t *itype) bool  { return t.TypeOf().Kind() == reflect.Ptr }
 
 func isEmptyInterface(t *itype) bool {
-	return t.cat == interfaceT && len(t.field) == 0
+	return t != nil && t.cat == interfaceT && len(t.field) == 0
 }
 
 func isGeneric(t *itype) bool {