🐛 bug: remove unbounded background goroutines from logger, cache, and internal storage memory

internal/storage/memory/memory.go

@@ -20,6 +20,7 @@ type Storage struct {
 	done       chan struct{}
 	gcInterval time.Duration
 	mux        sync.RWMutex
+	closeOnce  sync.Once
 }
 
 // Entry represents a value stored in memory along with its expiration.
@@ -38,7 +39,7 @@ func New(config ...Config) *Storage {
 	store := &Storage{
 		db:         make(map[string]Entry),
 		gcInterval: cfg.GCInterval,
-		done:       make(chan struct{}),
+		done:       make(chan struct{}, 1),
 	}
 
 	// Start garbage collector
@@ -150,7 +151,12 @@ func (s *Storage) ResetWithContext(ctx context.Context) error {
 // Close stops the background garbage collector and releases resources
 // associated with the storage instance.
 func (s *Storage) Close() error {
-	s.done <- struct{}{}
+	s.closeOnce.Do(func() {
+		select {
+		case s.done <- struct{}{}:
+		default:
+		}
+	})
 	return nil
 }
 

internal/storage/memory/memory_test.go

@@ -299,6 +299,25 @@ func Test_Storage_Memory_Close(t *testing.T) {
 	require.NoError(t, testStore.Close())
 }
 
+func Test_Storage_Memory_Close_Idempotent(t *testing.T) {
+	t.Parallel()
+
+	testStore := New()
+	require.NoError(t, testStore.Close())
+
+	errCh := make(chan error, 1)
+	go func() {
+		errCh <- testStore.Close()
+	}()
+
+	select {
+	case err := <-errCh:
+		require.NoError(t, err)
+	case <-time.After(time.Second):
+		t.Fatal("second Close blocked")
+	}
+}
+
 func Test_Storage_Memory_Conn(t *testing.T) {
 	t.Parallel()
 	testStore := New()

middleware/cache/cache.go

@@ -14,7 +14,6 @@ import (
 	"sort"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/gofiber/utils/v2"
@@ -24,11 +23,6 @@ import (
 	"github.com/gofiber/fiber/v3"
 )
 
-// timestampUpdatePeriod is the period which is used to check the cache expiration.
-// It should not be too long to provide more or less acceptable expiration error, and in the same
-// time it should not be too short to avoid overwhelming of the system
-const timestampUpdatePeriod = 300 * time.Millisecond
-
 // buffer size for hexpool
 const (
 	hexLen                       = sha256.Size * 2
@@ -148,11 +142,8 @@ func New(config ...Config) fiber.Handler {
 		}
 	}
 
-	var (
-		// Cache settings
-		mux       = &sync.RWMutex{}
-		timestamp = safeUnixSeconds(time.Now())
-	)
+	// Cache settings
+	mux := &sync.RWMutex{}
 	// Create manager to simplify storage operations ( see manager.go )
 	manager := newManager(cfg.Storage, redactKeys)
 	// Create indexed heap for tracking expirations ( see heap.go )
@@ -169,15 +160,6 @@ func New(config ...Config) fiber.Handler {
 	hashAuthorization := makeHashAuthFunc(hexBufPool)
 	buildVaryKey := makeBuildVaryKeyFunc(hexBufPool)
 
-	// Update timestamp in the configured interval
-	go func() {
-		ticker := time.NewTicker(timestampUpdatePeriod)
-		defer ticker.Stop()
-		for range ticker.C {
-			atomic.StoreUint64(&timestamp, safeUnixSeconds(time.Now()))
-		}
-	}()
-
 	// Delete key from both manager and storage
 	deleteKey := func(ctx context.Context, dkey string) error {
 		if err := manager.del(ctx, dkey); err != nil {
@@ -309,6 +291,9 @@ func New(config ...Config) fiber.Handler {
 			}
 		}
 
+		// Get timestamp before locking to keep the critical section small.
+		ts := safeUnixSeconds(time.Now())
+
 		// Lock entry
 		mux.Lock()
 		locked := true
@@ -324,9 +309,6 @@ func New(config ...Config) fiber.Handler {
 				locked = true
 			}
 		}
-		// Get timestamp
-		ts := atomic.LoadUint64(&timestamp)
-
 		// Cache Entry found
 		if e != nil {
 			entryAge = cachedResponseAge(e, ts)
@@ -815,7 +797,7 @@ func New(config ...Config) fiber.Handler {
 			return nil
 		}
 
-		ts = atomic.LoadUint64(&timestamp)
+		ts = safeUnixSeconds(time.Now())
 		responseTS := max(ts, nowUnix)
 
 		maxAgeSeconds := uint64(time.Duration(math.MaxInt64) / time.Second)

middleware/logger/logger.go

@@ -38,14 +38,29 @@ func New(config ...Config) fiber.Handler {
 	// Create correct timeformat
 	timestamp.Store(time.Now().In(cfg.timeZoneLocation).Format(cfg.TimeFormat))
 
-	// Update date/time every 500 milliseconds in a separate go routine
-	if strings.Contains(cfg.Format, "${"+TagTime+"}") {
-		go func() {
-			for {
-				time.Sleep(cfg.TimeInterval)
-				timestamp.Store(time.Now().In(cfg.timeZoneLocation).Format(cfg.TimeFormat))
+	timeEnabled := strings.Contains(cfg.Format, "${"+TagTime+"}")
+	var nextTimestampUpdate atomic.Pointer[time.Time]
+	if timeEnabled {
+		next := time.Now().Add(cfg.TimeInterval)
+		nextTimestampUpdate.Store(&next)
+	}
+
+	refreshTimestamp := func(now time.Time) {
+		if !timeEnabled {
+			return
+		}
+
+		for {
+			next := nextTimestampUpdate.Load()
+			if next != nil && now.Before(*next) {
+				return
 			}
-		}()
+			updated := now.Add(cfg.TimeInterval)
+			if nextTimestampUpdate.CompareAndSwap(next, &updated) {
+				timestamp.Store(now.In(cfg.timeZoneLocation).Format(cfg.TimeFormat))
+				return
+			}
+		}
 	}
 	// Set PID once
 	pid := strconv.Itoa(os.Getpid())
@@ -106,7 +121,10 @@ func New(config ...Config) fiber.Handler {
 		// no need for a reset, as long as we always override everything
 		data.Pid = pid
 		data.ErrPaddingStr = errPaddingStr
-		data.Timestamp = timestamp
+		if timeEnabled {
+			refreshTimestamp(time.Now())
+		}
+		data.Timestamp.Store(timestamp.Load())
 		// These compiled chains are shared across requests. The default logger and
 		// custom LoggerFunc implementations must only read them, for example via
 		// logtemplate.ExecuteChains.

middleware/logger/logger_test.go

@@ -908,6 +908,41 @@ func Test_Logger_Data_Race(t *testing.T) {
 	require.Equal(t, fiber.StatusOK, resp2.StatusCode)
 }
 
+func Test_Logger_TimeUpdatesAfterInterval(t *testing.T) {
+	t.Parallel()
+
+	var buf bytes.Buffer
+
+	app := fiber.New()
+	app.Use(New(Config{
+		Format:       "${time}",
+		TimeFormat:   time.RFC3339Nano,
+		TimeInterval: 10 * time.Millisecond,
+		Stream:       &buf,
+	}))
+	app.Get("/", func(c fiber.Ctx) error {
+		return c.SendStatus(fiber.StatusNoContent)
+	})
+
+	resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
+	require.NoError(t, err)
+	require.Equal(t, fiber.StatusNoContent, resp.StatusCode)
+	first := buf.String()
+	require.NotEmpty(t, first)
+
+	var second string
+	require.Eventually(t, func() bool {
+		buf.Reset()
+
+		resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
+		require.NoError(t, err)
+		require.Equal(t, fiber.StatusNoContent, resp.StatusCode)
+
+		second = buf.String()
+		return second != "" && second != first
+	}, 200*time.Millisecond, 5*time.Millisecond)
+}
+
 // go test -run Test_Response_Header
 func Test_Response_Header(t *testing.T) {
 	t.Parallel()