Skip to content

Commit

Permalink
fix: avoid memory leak in closure
Browse files Browse the repository at this point in the history
Simplify frame management. Remove node dependency on frame pointer.

Fixes #1618
  • Loading branch information
mvertes authored Apr 3, 2024
1 parent 2c92a7c commit 3fbebb3
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 43 deletions.
51 changes: 51 additions & 0 deletions _test/issue-1618.go
Original file line number Diff line number Diff line change
@@ -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
}
}
4 changes: 0 additions & 4 deletions interp/debugger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 3 additions & 8 deletions interp/interp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions interp/interp_consistent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 20 additions & 31 deletions interp/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:]
Expand All @@ -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)
Expand All @@ -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
}
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 3fbebb3

Please sign in to comment.