Skip to content

Commit af3868f

Browse files
committed
sync: release Pool memory during second and later GCs
Pool memory was only being released during the first GC after the first Put. Put assumes that p.local != nil means p is on the allPools list. poolCleanup (called during each GC) removed each pool from allPools but did not clear p.local, so each pool was cleared by exactly one GC and then never cleared again. This bug was introduced late in the Go 1.3 release cycle. Fixes #8979. LGTM=rsc R=golang-codereviews, bradfitz, r, rsc CC=golang-codereviews, khr https://golang.org/cl/162980043
1 parent 18051c0 commit af3868f

File tree

2 files changed

+35
-21
lines changed

2 files changed

+35
-21
lines changed

src/sync/pool.go

+2
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,8 @@ func poolCleanup() {
200200
}
201201
l.shared = nil
202202
}
203+
p.local = nil
204+
p.localSize = 0
203205
}
204206
allPools = []*Pool{}
205207
}

src/sync/pool_test.go

+33-21
Original file line numberDiff line numberDiff line change
@@ -69,32 +69,44 @@ func TestPoolNew(t *testing.T) {
6969
}
7070
}
7171

72-
// Test that Pool does not hold pointers to previously cached
73-
// resources
72+
// Test that Pool does not hold pointers to previously cached resources.
7473
func TestPoolGC(t *testing.T) {
74+
testPool(t, true)
75+
}
76+
77+
// Test that Pool releases resources on GC.
78+
func TestPoolRelease(t *testing.T) {
79+
testPool(t, false)
80+
}
81+
82+
func testPool(t *testing.T, drain bool) {
7583
var p Pool
76-
var fin uint32
7784
const N = 100
78-
for i := 0; i < N; i++ {
79-
v := new(string)
80-
runtime.SetFinalizer(v, func(vv *string) {
81-
atomic.AddUint32(&fin, 1)
82-
})
83-
p.Put(v)
84-
}
85-
for i := 0; i < N; i++ {
86-
p.Get()
87-
}
88-
for i := 0; i < 5; i++ {
89-
runtime.GC()
90-
time.Sleep(time.Duration(i*100+10) * time.Millisecond)
91-
// 1 pointer can remain on stack or elsewhere
92-
if atomic.LoadUint32(&fin) >= N-1 {
93-
return
85+
loop:
86+
for try := 0; try < 3; try++ {
87+
var fin, fin1 uint32
88+
for i := 0; i < N; i++ {
89+
v := new(string)
90+
runtime.SetFinalizer(v, func(vv *string) {
91+
atomic.AddUint32(&fin, 1)
92+
})
93+
p.Put(v)
94+
}
95+
if drain {
96+
for i := 0; i < N; i++ {
97+
p.Get()
98+
}
99+
}
100+
for i := 0; i < 5; i++ {
101+
runtime.GC()
102+
time.Sleep(time.Duration(i*100+10) * time.Millisecond)
103+
// 1 pointer can remain on stack or elsewhere
104+
if fin1 = atomic.LoadUint32(&fin); fin1 >= N-1 {
105+
continue loop
106+
}
94107
}
108+
t.Fatalf("only %v out of %v resources are finalized on try %v", fin1, N, try)
95109
}
96-
t.Fatalf("only %v out of %v resources are finalized",
97-
atomic.LoadUint32(&fin), N)
98110
}
99111

100112
func TestPoolStress(t *testing.T) {

0 commit comments

Comments
 (0)