Skip to content

Commit 513b877

Browse files
committed
Fix memory leak in case cached data forms a reference cycle
sc now requires Go 1.24+ since this is dependent on weak package. See issue #8.
1 parent bdfc3d5 commit 513b877

File tree

5 files changed

+54
-22
lines changed

5 files changed

+54
-22
lines changed

.github/workflows/main.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
strategy:
1414
matrix:
1515
go:
16-
- "1.22"
16+
- "1.24"
1717
steps:
1818
- uses: actions/checkout@v3
1919
- uses: actions/setup-go@v3
@@ -30,7 +30,7 @@ jobs:
3030
strategy:
3131
matrix:
3232
go:
33-
- "1.22"
33+
- "1.24"
3434
steps:
3535
- uses: actions/checkout@v3
3636
- uses: actions/setup-go@v3
@@ -47,7 +47,7 @@ jobs:
4747
strategy:
4848
matrix:
4949
go:
50-
- "1.22"
50+
- "1.24"
5151
env:
5252
GOCACHE: "/tmp/go/cache"
5353
steps:
@@ -81,7 +81,7 @@ jobs:
8181
strategy:
8282
matrix:
8383
go:
84-
- "1.22"
84+
- "1.24"
8585
steps:
8686
- uses: actions/checkout@v3
8787
with:

cache_test.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1170,6 +1170,28 @@ func TestCleaningCache(t *testing.T) {
11701170
func TestCleaningCacheFinalizer(t *testing.T) {
11711171
t.Parallel()
11721172

1173+
for _, c := range allCaches(10) {
1174+
c := c
1175+
t.Run(c.name, func(t *testing.T) {
1176+
t.Parallel()
1177+
1178+
replaceFn := func(_ context.Context, _ struct{}) (string, error) { return "value", nil }
1179+
ca, err := New(replaceFn, time.Hour, time.Hour, append(c.cacheOpts, WithCleanupInterval(time.Second))...)
1180+
assert.NoError(t, err)
1181+
1182+
_, _ = ca.Get(context.Background(), struct{}{})
1183+
runtime.KeepAlive(ca)
1184+
runtime.GC() // finalizer is called and cleaner is stopped
1185+
time.Sleep(1 * time.Second)
1186+
})
1187+
}
1188+
}
1189+
1190+
// TestCleaningCacheFinalizer_ReferenceCycle is like TestCleaningCacheFinalizer, but tests case when cached data contains
1191+
// a reference to *Cache itself.
1192+
func TestCleaningCacheFinalizer_ReferenceCycle(t *testing.T) {
1193+
t.Parallel()
1194+
11731195
for _, c := range allCaches(10) {
11741196
c := c
11751197
t.Run(c.name, func(t *testing.T) {
@@ -1187,8 +1209,9 @@ func TestCleaningCacheFinalizer(t *testing.T) {
11871209
assert.NoError(t, err)
11881210

11891211
_, _ = ca.Get(context.Background(), struct{}{})
1190-
runtime.KeepAlive(c)
1212+
runtime.KeepAlive(ca)
11911213
runtime.GC() // finalizer is called and cleaner is stopped
1214+
time.Sleep(1 * time.Second)
11921215
})
11931216
}
11941217
}

cleaner.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package sc
33
import (
44
"runtime"
55
"time"
6+
"weak"
67
)
78

89
// cleaner is launched as a single goroutine to regularly clean up expired items from the cache.
@@ -11,16 +12,24 @@ import (
1112
// See https://github.com/patrickmn/go-cache/blob/46f407853014144407b6c2ec7ccc76bf67958d93/cache.go#L1115 for more on this design.
1213
type cleaner[K comparable, V any] struct {
1314
closer chan struct{}
14-
c *cache[K, V]
15+
// We use weak pointer here in order to deal with an extremely-unlikely case where cached data itself
16+
// somehow has a reference to *Cache itself, forming a reference cycle.
17+
// If above is the case, and we're using strong reference here, the cleaner goroutine keeps a reference to this
18+
// reference cycle, therefore *Cache and *cache never being evicted, leading to memory leaks.
19+
//
20+
// This case can be tested in TestCleaningCacheFinalizer, where testers can manually check that stopCleaner function
21+
// is called, for example by adding `fmt.Println("cleanup called")` there.
22+
c weak.Pointer[cache[K, V]]
1523
}
1624

1725
func startCleaner[K comparable, V any](c *Cache[K, V], interval time.Duration) {
26+
closer := make(chan struct{})
1827
cl := &cleaner[K, V]{
19-
closer: make(chan struct{}),
20-
c: c.cache,
28+
closer: closer,
29+
c: weak.Make(c.cache),
2130
}
2231
go cl.run(interval)
23-
runtime.SetFinalizer(c, stopCleaner(cl))
32+
runtime.AddCleanup(c, stopCleaner, closer)
2433
}
2534

2635
func (cl *cleaner[K, V]) run(interval time.Duration) {
@@ -29,19 +38,17 @@ func (cl *cleaner[K, V]) run(interval time.Duration) {
2938
for {
3039
select {
3140
case <-ticker.C:
32-
cl.c.cleanup()
41+
c := cl.c.Value()
42+
if c == nil {
43+
return
44+
}
45+
c.cleanup()
3346
case <-cl.closer:
3447
return
3548
}
3649
}
3750
}
3851

39-
func (cl *cleaner[K, V]) stop() {
40-
cl.closer <- struct{}{}
41-
}
42-
43-
func stopCleaner[K comparable, V any](cl *cleaner[K, V]) func(*Cache[K, V]) {
44-
return func(_ *Cache[K, V]) {
45-
cl.stop()
46-
}
52+
func stopCleaner(closer chan<- struct{}) {
53+
close(closer)
4754
}

go.mod

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
module github.com/motoki317/sc
22

3-
go 1.21
3+
go 1.24
44

5-
require github.com/stretchr/testify v1.9.0
5+
toolchain go1.24.1
6+
7+
require github.com/stretchr/testify v1.10.0
68

79
require (
810
github.com/davecgh/go-spew v1.1.1 // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
99
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
1010
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
1111
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
12-
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
13-
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
12+
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
13+
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
1414
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
1515
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
1616
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=

0 commit comments

Comments
 (0)