Skip to content

Commit 3fbebb3

Browse files
mvertesmvertes
authored
fix: avoid memory leak in closure
Simplify frame management. Remove node dependency on frame pointer. Fixes #1618
1 parent 2c92a7c commit 3fbebb3

File tree

5 files changed

+75
-43
lines changed

5 files changed

+75
-43
lines changed

‎_test/issue-1618.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"runtime"
6+
"sync"
7+
)
8+
9+
func humanizeBytes(bytes uint64) string {
10+
const (
11+
_ = iota
12+
kB uint64 = 1 << (10 * iota)
13+
mB
14+
gB
15+
tB
16+
pB
17+
)
18+
19+
switch {
20+
case bytes < kB:
21+
return fmt.Sprintf("%dB", bytes)
22+
case bytes < mB:
23+
return fmt.Sprintf("%.2fKB", float64(bytes)/float64(kB))
24+
case bytes < gB:
25+
return fmt.Sprintf("%.2fMB", float64(bytes)/float64(mB))
26+
case bytes < tB:
27+
return fmt.Sprintf("%.2fGB", float64(bytes)/float64(gB))
28+
case bytes < pB:
29+
return fmt.Sprintf("%.2fTB", float64(bytes)/float64(tB))
30+
default:
31+
return fmt.Sprintf("%dB", bytes)
32+
}
33+
}
34+
35+
func main() {
36+
i := 0
37+
wg := sync.WaitGroup{}
38+
39+
for {
40+
var m runtime.MemStats
41+
runtime.ReadMemStats(&m)
42+
fmt.Printf("#%d: alloc = %s, routines = %d, gc = %d\n", i, humanizeBytes(m.Alloc), runtime.NumGoroutine(), m.NumGC)
43+
44+
wg.Add(1)
45+
go func() {
46+
wg.Done()
47+
}()
48+
wg.Wait()
49+
i = i + 1
50+
}
51+
}

‎interp/debugger.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,6 @@ func (dbg *Debugger) enterCall(nFunc, nCall *node, f *frame) {
272272
switch nFunc.kind {
273273
case funcLit:
274274
f.debug.kind = frameCall
275-
if nFunc.frame != nil {
276-
nFunc.frame.debug.kind = frameClosure
277-
nFunc.frame.debug.node = nFunc
278-
}
279275

280276
case funcDecl:
281277
f.debug.kind = frameCall

‎interp/interp.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ type node struct {
3333
tnext *node // true branch successor (CFG)
3434
fnext *node // false branch successor (CFG)
3535
interp *Interpreter // interpreter context
36-
frame *frame // frame pointer used for closures only (TODO: suppress this)
3736
index int64 // node index (dot display)
3837
findex int // index of value in frame or frame size (func def, type def)
3938
level int // number of frame indirections to access value
@@ -138,7 +137,7 @@ func newFrame(anc *frame, length int, id uint64) *frame {
138137

139138
func (f *frame) runid() uint64 { return atomic.LoadUint64(&f.id) }
140139
func (f *frame) setrunid(id uint64) { atomic.StoreUint64(&f.id, id) }
141-
func (f *frame) clone(fork bool) *frame {
140+
func (f *frame) clone() *frame {
142141
f.mutex.RLock()
143142
defer f.mutex.RUnlock()
144143
nf := &frame{
@@ -150,12 +149,8 @@ func (f *frame) clone(fork bool) *frame {
150149
done: f.done,
151150
debug: f.debug,
152151
}
153-
if fork {
154-
nf.data = make([]reflect.Value, len(f.data))
155-
copy(nf.data, f.data)
156-
} else {
157-
nf.data = f.data
158-
}
152+
nf.data = make([]reflect.Value, len(f.data))
153+
copy(nf.data, f.data)
159154
return nf
160155
}
161156

‎interp/interp_consistent_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ func TestInterpConsistencyBuild(t *testing.T) {
8080
file.Name() == "time0.go" || // display time (similar to random number)
8181
file.Name() == "factor.go" || // bench
8282
file.Name() == "fib.go" || // bench
83+
file.Name() == "issue-1618.go" || // bench (infinite running)
8384

8485
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
8586
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‎

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -988,9 +988,6 @@ func genFunctionWrapper(n *node) func(*frame) reflect.Value {
988988
funcType := n.typ.TypeOf()
989989

990990
return func(f *frame) reflect.Value {
991-
if n.frame != nil { // Use closure context if defined.
992-
f = n.frame
993-
}
994991
return reflect.MakeFunc(funcType, func(in []reflect.Value) []reflect.Value {
995992
// Allocate and init local frame. All values to be settable and addressable.
996993
fr := newFrame(f, len(def.types), f.runid())
@@ -1292,14 +1289,13 @@ func call(n *node) {
12921289
}
12931290

12941291
n.exec = func(f *frame) bltn {
1295-
var def *node
1296-
var ok bool
1297-
1292+
f.mutex.Lock()
12981293
bf := value(f)
1299-
1300-
if def, ok = bf.Interface().(*node); ok {
1294+
def, ok := bf.Interface().(*node)
1295+
if ok {
13011296
bf = def.rval
13021297
}
1298+
f.mutex.Unlock()
13031299

13041300
// Call bin func if defined
13051301
if bf.IsValid() {
@@ -1343,12 +1339,7 @@ func call(n *node) {
13431339
return tnext
13441340
}
13451341

1346-
anc := f
1347-
// Get closure frame context (if any)
1348-
if def.frame != nil {
1349-
anc = def.frame
1350-
}
1351-
nf := newFrame(anc, len(def.types), anc.runid())
1342+
nf := newFrame(f, len(def.types), f.runid())
13521343
var vararg reflect.Value
13531344

13541345
// Init return values
@@ -1887,27 +1878,22 @@ func getIndexMap2(n *node) {
18871878
}
18881879
}
18891880

1890-
const fork = true // Duplicate frame in frame.clone().
1891-
18921881
// getFunc compiles a closure function generator for anonymous functions.
18931882
func getFunc(n *node) {
18941883
i := n.findex
18951884
l := n.level
18961885
next := getExec(n.tnext)
1886+
numRet := len(n.typ.ret)
18971887

18981888
n.exec = func(f *frame) bltn {
1899-
fr := f.clone(fork)
1900-
nod := *n
1901-
nod.val = &nod
1902-
nod.frame = fr
1903-
def := &nod
1904-
numRet := len(def.typ.ret)
1889+
fr := f.clone()
1890+
o := getFrame(f, l).data[i]
19051891

1906-
fct := reflect.MakeFunc(nod.typ.TypeOf(), func(in []reflect.Value) []reflect.Value {
1892+
fct := reflect.MakeFunc(n.typ.TypeOf(), func(in []reflect.Value) []reflect.Value {
19071893
// Allocate and init local frame. All values to be settable and addressable.
1908-
fr2 := newFrame(fr, len(def.types), fr.runid())
1894+
fr2 := newFrame(fr, len(n.types), fr.runid())
19091895
d := fr2.data
1910-
for i, t := range def.types {
1896+
for i, t := range n.types {
19111897
d[i] = reflect.New(t).Elem()
19121898
}
19131899
d = d[numRet:]
@@ -1918,7 +1904,7 @@ func getFunc(n *node) {
19181904
// In case of unused arg, there may be not even a frame entry allocated, just skip.
19191905
break
19201906
}
1921-
typ := def.typ.arg[i]
1907+
typ := n.typ.arg[i]
19221908
switch {
19231909
case isEmptyInterface(typ) || typ.TypeOf() == valueInterfaceType:
19241910
d[i].Set(arg)
@@ -1930,12 +1916,19 @@ func getFunc(n *node) {
19301916
}
19311917

19321918
// Interpreter code execution.
1933-
runCfg(def.child[3].start, fr2, def, n)
1919+
runCfg(n.child[3].start, fr2, n, n)
1920+
1921+
f.mutex.Lock()
1922+
getFrame(f, l).data[i] = o
1923+
f.mutex.Unlock()
19341924

19351925
return fr2.data[:numRet]
19361926
})
19371927

1928+
f.mutex.Lock()
19381929
getFrame(f, l).data[i] = fct
1930+
f.mutex.Unlock()
1931+
19391932
return next
19401933
}
19411934
}
@@ -1946,11 +1939,9 @@ func getMethod(n *node) {
19461939
next := getExec(n.tnext)
19471940

19481941
n.exec = func(f *frame) bltn {
1949-
fr := f.clone(!fork)
19501942
nod := *(n.val.(*node))
19511943
nod.val = &nod
19521944
nod.recv = n.recv
1953-
nod.frame = fr
19541945
getFrame(f, l).data[i] = genFuncValue(&nod)(f)
19551946
return next
19561947
}
@@ -2021,11 +2012,9 @@ func getMethodByName(n *node) {
20212012
panic(n.cfgErrorf("method not found: %s", name))
20222013
}
20232014

2024-
fr := f.clone(!fork)
20252015
nod := *m
20262016
nod.val = &nod
20272017
nod.recv = &receiver{nil, val.value, li}
2028-
nod.frame = fr
20292018
getFrame(f, l).data[i] = genFuncValue(&nod)(f)
20302019
return next
20312020
}

0 commit comments

Comments
 (0)