From 3f396c5666ef01ea621c89213d7f57db5340ac26 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 19 Aug 2026 23:40:16 -0400 Subject: [PATCH] fix(plugins): stop the cache janitor when a plugin cache is dropped newCacheService started a ttlcache janitor goroutine that only stopped via the explicit Close() path, so a cache service that was discarded without being closed leaked its janitor for the process lifetime. It now registers the same runtime.AddCleanup safety net that utils/cache.simpleCache already uses. --- plugins/host_cache.go | 10 +++++++++- plugins/host_cache_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/plugins/host_cache.go b/plugins/host_cache.go index b90d790cf..f410dfd3d 100644 --- a/plugins/host_cache.go +++ b/plugins/host_cache.go @@ -2,6 +2,7 @@ package plugins import ( "context" + "runtime" "time" "github.com/jellydator/ttlcache/v3" @@ -29,11 +30,18 @@ func newCacheService(pluginName string) *cacheServiceImpl { // Start the janitor goroutine to clean up expired entries go cache.Start() - return &cacheServiceImpl{ + svc := &cacheServiceImpl{ pluginName: pluginName, cache: cache, defaultTTL: defaultCacheTTL, } + + // Automatic cleanup to prevent goroutine leak when the service is garbage collected + runtime.AddCleanup(svc, func(ttlCache *ttlcache.Cache[string, any]) { + ttlCache.Stop() + }, cache) + + return svc } // getTTL converts seconds to a duration, using default if 0 or negative diff --git a/plugins/host_cache_test.go b/plugins/host_cache_test.go index cf3973fc4..e3dbf6946 100644 --- a/plugins/host_cache_test.go +++ b/plugins/host_cache_test.go @@ -8,9 +8,11 @@ import ( "encoding/hex" "encoding/json" "errors" + "fmt" "net/http" "os" "path/filepath" + "runtime" "time" "github.com/navidrome/navidrome/conf" @@ -599,3 +601,39 @@ var _ = Describe("CacheService Integration", Ordered, func() { }) }) }) + +var _ = Describe("newCacheService", func() { + // Background goroutines wind down late, so poll for two consecutive equal samples. + settleGoroutines := func() int { + prev := -1 + for range 100 { + runtime.GC() + n := runtime.NumGoroutine() + if n == prev { + return n + } + prev = n + time.Sleep(10 * time.Millisecond) + } + return prev + } + + It("stops the janitor goroutine once the service is unreachable", func() { + const numServices = 20 + + baseline := settleGoroutines() + + services := make([]*cacheServiceImpl, 0, numServices) + for i := range numServices { + services = append(services, newCacheService(fmt.Sprintf("plugin_%d", i))) + } + Expect(runtime.NumGoroutine()).To(BeNumerically(">=", baseline+numServices), + "expected one janitor goroutine per cache service") + + services = nil + Expect(services).To(BeNil()) + + Expect(settleGoroutines()).To(BeNumerically("<", baseline+numServices), + "janitor goroutines leaked after the services became unreachable") + }) +})