diff --git a/internal/lru/lru.go b/internal/lru/lru.go index fbb3b40..9222de4 100644 --- a/internal/lru/lru.go +++ b/internal/lru/lru.go @@ -74,8 +74,9 @@ func NewLRU[K comparable, V any](size int, onEvict EvictCallback[K, V], ttl time // enable deleteExpired() running in separate goroutine for cache with non-zero TTL // - // Important: done channel is never closed, so deleteExpired() goroutine will never exit, - // it's decided to add functionality to close it in the version later than v2. + // The goroutine runs until Close is called, which closes the done channel and + // signals it to exit. Callers that discard the cache should call Close to avoid + // leaking the goroutine. if res.ttl != noEvictionTTL { go func(done <-chan struct{}) { ticker := time.NewTicker(res.ttl / numBuckets) @@ -290,17 +291,19 @@ func (c *LRU[K, V]) Resize(size int) (evicted int) { return diff } -// Close destroys cleanup goroutine. To clean up the cache, run Purge() before Close(). -// func (c *LRU[K, V]) Close() { -// c.mu.Lock() -// defer c.mu.Unlock() -// select { -// case <-c.done: -// return -// default: -// } -// close(c.done) -// } +// Close stops the background cleanup goroutine and releases its resources. +// Close is safe to call multiple times and from multiple goroutines. +// To also drop cached entries, call Purge before Close. +func (c *LRU[K, V]) Close() { + c.mu.Lock() + defer c.mu.Unlock() + select { + case <-c.done: + return + default: + } + close(c.done) +} // removeOldest removes the oldest item from the cache. Has to be called with lock! func (c *LRU[K, V]) removeOldest() { diff --git a/internal/stmt_store/stmt_store.go b/internal/stmt_store/stmt_store.go index 6f13ebb..b593e2b 100644 --- a/internal/stmt_store/stmt_store.go +++ b/internal/stmt_store/stmt_store.go @@ -68,6 +68,11 @@ type Store interface { // Parameters: // key: The key associated with the Stmt object to be deleted. Delete(key string) + + // Close releases resources held by the store, stopping any background + // maintenance goroutines started for cache expiration. It is safe to call + // Close multiple times. The store should not be used after Close is called. + Close() } // defaultMaxSize defines the default maximum capacity of the cache. @@ -137,6 +142,11 @@ func (s *lruStore) Delete(key string) { s.lru.Remove(key) } +// Close stops the underlying LRU cache's background expiration goroutine. +func (s *lruStore) Close() { + s.lru.Close() +} + type ConnPool interface { PrepareContext(ctx context.Context, query string) (*sql.Stmt, error) } diff --git a/prepare_stmt.go b/prepare_stmt.go index 799df5b..762fc99 100644 --- a/prepare_stmt.go +++ b/prepare_stmt.go @@ -48,7 +48,9 @@ func (db *PreparedStmtDB) GetDBConn() (*sql.DB, error) { return nil, ErrInvalidDB } -// Close closes all prepared statements in the store +// Close closes all prepared statements in the store and stops the store's +// background maintenance goroutine to avoid leaking it for the lifetime of the +// process. func (db *PreparedStmtDB) Close() { db.Mux.Lock() defer db.Mux.Unlock() @@ -56,6 +58,7 @@ func (db *PreparedStmtDB) Close() { for _, key := range db.Stmts.Keys() { db.Stmts.Delete(key) } + db.Stmts.Close() } // Reset Deprecated use Close instead diff --git a/tests/lru_test.go b/tests/lru_test.go index 4be94ae..d94905c 100644 --- a/tests/lru_test.go +++ b/tests/lru_test.go @@ -6,6 +6,7 @@ import ( "math" "math/big" "reflect" + "runtime" "sync" "testing" "time" @@ -13,6 +14,49 @@ import ( "gorm.io/gorm/internal/lru" ) +// TestLRU_Close_StopsBackgroundGoroutine verifies that Close terminates the +// background deleteExpired goroutine started for a cache with a non-zero TTL. +// Without Close, every such cache leaks one goroutine for the lifetime of the +// process (see go-gorm/gorm issue #7788). +func TestLRU_Close_StopsBackgroundGoroutine(t *testing.T) { + const n = 100 + + runtime.GC() + time.Sleep(50 * time.Millisecond) + baseline := runtime.NumGoroutine() + + caches := make([]*lru.LRU[string, int], 0, n) + for i := 0; i < n; i++ { + // A non-zero TTL starts a background deleteExpired goroutine per cache. + caches = append(caches, lru.NewLRU[string, int](16, nil, time.Minute)) + } + + // Sanity check: the background goroutines are running. + if got := runtime.NumGoroutine(); got < baseline+n/2 { + t.Fatalf("expected background goroutines to start: got %d, baseline %d", got, baseline) + } + + for _, c := range caches { + c.Close() + } + caches = nil + + // After Close the background goroutines must terminate and the goroutine + // count must return close to the baseline. + deadline := time.Now().Add(2 * time.Second) + for { + runtime.GC() + got := runtime.NumGoroutine() + if got <= baseline+n/10 { + return + } + if time.Now().After(deadline) { + t.Fatalf("background goroutines leaked after Close: got %d, baseline %d", got, baseline) + } + time.Sleep(20 * time.Millisecond) + } +} + func TestLRU_Add_ExistingKey_UpdatesValueAndExpiresAt(t *testing.T) { lru := lru.NewLRU[string, int](10, nil, time.Hour) lru.Add("key1", 1)