Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid memory leak in closure #1621

Merged
merged 2 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading