Skip to content

Commit

Permalink
sync: release Pool memory during second and later GCs
Browse files Browse the repository at this point in the history
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 golang#8979.

LGTM=rsc
R=golang-codereviews, bradfitz, r, rsc
CC=golang-codereviews, khr
https://golang.org/cl/162980043
  • Loading branch information
dvyukov authored and wheatman committed Jul 9, 2018
1 parent bec9db8 commit f40045a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 21 deletions.
2 changes: 2 additions & 0 deletions src/sync/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ func poolCleanup() {
}
l.shared = nil
}
p.local = nil
p.localSize = 0
}
allPools = []*Pool{}
}
Expand Down
54 changes: 33 additions & 21 deletions src/sync/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,32 +69,44 @@ func TestPoolNew(t *testing.T) {
}
}

// Test that Pool does not hold pointers to previously cached
// resources
// Test that Pool does not hold pointers to previously cached resources.
func TestPoolGC(t *testing.T) {
testPool(t, true)
}

// Test that Pool releases resources on GC.
func TestPoolRelease(t *testing.T) {
testPool(t, false)
}

func testPool(t *testing.T, drain bool) {
var p Pool
var fin uint32
const N = 100
for i := 0; i < N; i++ {
v := new(string)
runtime.SetFinalizer(v, func(vv *string) {
atomic.AddUint32(&fin, 1)
})
p.Put(v)
}
for i := 0; i < N; i++ {
p.Get()
}
for i := 0; i < 5; i++ {
runtime.GC()
time.Sleep(time.Duration(i*100+10) * time.Millisecond)
// 1 pointer can remain on stack or elsewhere
if atomic.LoadUint32(&fin) >= N-1 {
return
loop:
for try := 0; try < 3; try++ {
var fin, fin1 uint32
for i := 0; i < N; i++ {
v := new(string)
runtime.SetFinalizer(v, func(vv *string) {
atomic.AddUint32(&fin, 1)
})
p.Put(v)
}
if drain {
for i := 0; i < N; i++ {
p.Get()
}
}
for i := 0; i < 5; i++ {
runtime.GC()
time.Sleep(time.Duration(i*100+10) * time.Millisecond)
// 1 pointer can remain on stack or elsewhere
if fin1 = atomic.LoadUint32(&fin); fin1 >= N-1 {
continue loop
}
}
t.Fatalf("only %v out of %v resources are finalized on try %v", fin1, N, try)
}
t.Fatalf("only %v out of %v resources are finalized",
atomic.LoadUint32(&fin), N)
}

func TestPoolStress(t *testing.T) {
Expand Down

0 comments on commit f40045a

Please sign in to comment.