Skip to content

Commit d295b3c

Browse files
committed
fix: prevent ticker leaks and correct worker pool resize behavior
Stop time.Tickers in background loops on shutdown to avoid resource leaks WorkerPool: send only required (-diff) quit signals on shrink and buffer quit channel to reduce blocking Includes: hypercache.go ticker lifecycle cleanup, pool.go resize logic and worker loop refactor.
1 parent 266a010 commit d295b3c

3 files changed

Lines changed: 68 additions & 27 deletions

File tree

PRD.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# PRD
2+
3+
Overview
4+
Hypercache is a thread‑safe caching library with pluggable backends and eviction algorithms, already targeting Go 1.25 and generics. The design exposes a service interface that supports middleware layers for decorating cache operations.
5+
6+
Findings & Suggestions
7+
Ticker Lifecycle – Background jobs create time.Tickers but never stop them, which can leak resources; explicitly call tick.Stop() when terminating the loops.
8+
9+
WorkerPool Resize Logic – Resizing the pool sends a quit signal pool.workers times regardless of how many workers need removal; adjust to send -diff signals and consider buffering to avoid blocking.
10+
11+
Expired‑Key Semantics – GetMultiple returns ErrKeyExpired, yet tests expect ErrKeyNotFound for expired items, indicating a behavior mismatch or test expectation issue.
12+
13+
Per‑Call Goroutines – Get and GetWithInfo spawn a goroutine for each expired item, which may create overhead under heavy load; consider synchronous cleanup or a dedicated worker to batch expirations.
14+
15+
Object Pool Hygiene – ItemPoolManager returns items to the pool without resetting fields, risking retention of large values; zero the struct before reusing it.
16+
17+
Modern Go Features – The project already uses Go 1.25; further modernization could include:
18+
19+
Generics for typed values rather than any, providing compile‑time safety.
20+
21+
Use of maps.Clone/maps.DeleteFunc and slices.SortFunc for cleaner collection handling.
22+
23+
errors.Is/errors.Join to simplify error comparisons.
24+
25+
Potential Enhancements
26+
27+
Expose a MGet/MSet on backends for efficient multi-key operations.
28+
29+
Provide context-aware cancellation in WorkerPool and background jobs to ease shutdown.
30+
31+
Add methods such as Contains, Peek, Increment, or TTL refresh APIs.
32+
33+
Integrate OpenTelemetry metrics and tracing for observability.

hypercache.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,9 @@ func (hyperCache *HyperCache[T]) startBackgroundJobs() {
210210
// trigger eviction
211211
hyperCache.evictionLoop(ctx)
212212
case <-hyperCache.stop:
213-
// stop the loops
213+
// stop the loops and ticker to avoid leaks
214+
tick.Stop()
215+
214216
return
215217
}
216218
}
@@ -228,6 +230,9 @@ func (hyperCache *HyperCache[T]) startBackgroundJobs() {
228230
case <-tick.C:
229231
hyperCache.evictionLoop(ctx)
230232
case <-hyperCache.stop:
233+
// stop ticker to avoid leaks
234+
tick.Stop()
235+
231236
return
232237
}
233238
}

pool.go

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ type WorkerPool struct {
1919
// NewWorkerPool creates a new worker pool with the given number of workers.
2020
func NewWorkerPool(workers int) *WorkerPool {
2121
pool := &WorkerPool{
22-
workers: workers,
23-
jobs: make(chan JobFunc, workers),
24-
quit: make(chan struct{}),
22+
workers: workers,
23+
jobs: make(chan JobFunc, workers),
24+
// buffer quit to allow multiple resize signals without blocking immediately
25+
quit: make(chan struct{}, workers),
2526
errorChan: make(chan error, workers),
2627
}
2728
pool.start()
@@ -65,18 +66,12 @@ func (pool *WorkerPool) Resize(newSize int) {
6566
if diff > 0 {
6667
// Increase the number of workers
6768
for range diff {
68-
go func() {
69-
for job := range pool.jobs {
70-
err := job()
71-
if err != nil {
72-
pool.errorChan <- err
73-
}
74-
}
75-
}()
69+
go pool.worker()
7670
}
7771
} else {
7872
// Decrease the number of workers
79-
for range pool.workers {
73+
// Send only the number of quit signals needed to remove workers
74+
for range diff {
8075
pool.quit <- struct{}{}
8176
}
8277
}
@@ -85,20 +80,28 @@ func (pool *WorkerPool) Resize(newSize int) {
8580
// start starts the worker pool.
8681
func (pool *WorkerPool) start() {
8782
for range pool.workers {
88-
go func() {
89-
for {
90-
select {
91-
case job := <-pool.jobs:
92-
err := job()
93-
if err != nil {
94-
pool.errorChan <- err
95-
}
96-
97-
pool.wg.Done()
98-
case <-pool.quit:
99-
return
100-
}
83+
go pool.worker()
84+
}
85+
}
86+
87+
// worker is the main loop executed by each worker goroutine.
88+
func (pool *WorkerPool) worker() {
89+
for {
90+
select {
91+
case job := <-pool.jobs:
92+
if job == nil {
93+
// jobs channel closed
94+
return
10195
}
102-
}()
96+
97+
err := job()
98+
if err != nil {
99+
pool.errorChan <- err
100+
}
101+
102+
pool.wg.Done()
103+
case <-pool.quit:
104+
return
105+
}
103106
}
104107
}

0 commit comments

Comments
 (0)