Make cache safe and bounded (thread-safe + eviction + cleanup) #12

New Issue

peio · 2026-01-26T00:27:09Z

peio commented

2026-01-26 00:27:09 +00:00

Problem

The in-memory cache used in internal/mailcloak/policy.go (see around policy.go:14) is currently not concurrency-safe and appears unbounded.

This can lead to:

Data races under concurrent policy requests (Postfix policy daemon is typically concurrent).
Unbounded memory growth over time (potentially a slow memory leak pattern).
Stale entries remaining forever if there is no TTL / cleanup.

Goal

Make the cache:

Concurrency-safe (no races).
Bounded (max size + eviction).
Self-maintained (periodic cleanup of expired entries / stale keys).

Proposed changes

Concurrency safety

Pick one:

Protect the cache map with a sync.RWMutex (preferred if we implement LRU since we’ll need structured operations anyway).
Or replace with sync.Map (but LRU/eviction is harder to do correctly with sync.Map, so likely not the best fit if we add LRU).

Size limits + eviction

Implement a bounded cache with eviction, ideally LRU:

Configurable maxEntries (default value if config not provided).
On insert when size exceeds maxEntries, evict the least-recently-used entries.
Consider TTL per entry (optional but recommended) to prevent stale data.

Implementation options:

Small internal LRU (map + doubly-linked list via container/list)
Or use a well-known Go LRU library (only if adding a dependency is acceptable).

Periodic cleanup

Add a periodic cleanup mechanism:

A background goroutine with a time.Ticker (e.g. every N minutes) that prunes expired entries.
Ensure it stops cleanly on shutdown (context cancellation / stop channel).

Acceptance criteria

✅ go test -race ./... shows no data races under load.
✅ Cache cannot exceed maxEntries (validated by unit tests).
✅ Eviction behaves like LRU (unit tests: access updates recency, insert triggers expected eviction).
✅ Periodic cleanup removes expired entries (unit test with fake clock or controllable time source if possible).
✅ No goroutine leaks (cleanup ticker stops on shutdown).

Notes / hints

If LRU is implemented, operations typically require a lock anyway → sync.RWMutex + explicit LRU structure is straightforward.
If the cache is read-heavy, consider RLock for reads, but note that LRU “touch” on read requires upgrading to write; simplest approach is just a single Lock for Get/Set if performance is not critical.

Tasks checklist

Identify cache structure in policy.go (around line 14) and all access points.
Add locking around cache operations (or refactor to a dedicated cache type).
Implement bounded LRU (maxEntries) and eviction.
Add TTL metadata and periodic cleanup.
Add tests + run -race.

# Problem The in-memory cache used in `internal/mailcloak/policy.go` (see around `policy.go:14`) is currently **not concurrency-safe** and appears **unbounded**. This can lead to: - **Data races** under concurrent policy requests (Postfix policy daemon is typically concurrent). - **Unbounded memory growth** over time (potentially a slow memory leak pattern). - Stale entries remaining forever if there is no TTL / cleanup. # Goal Make the cache: 1. **Concurrency-safe** (no races). 2. **Bounded** (max size + eviction). 3. **Self-maintained** (periodic cleanup of expired entries / stale keys). # Proposed changes 1) **Concurrency safety** Pick one: - Protect the cache map with a `sync.RWMutex` (preferred if we implement LRU since we’ll need structured operations anyway). - Or replace with `sync.Map` (but LRU/eviction is harder to do correctly with `sync.Map`, so likely not the best fit if we add LRU). 2) **Size limits + eviction** Implement a bounded cache with eviction, ideally **LRU**: - Configurable `maxEntries` (default value if config not provided). - On insert when size exceeds `maxEntries`, evict the least-recently-used entries. - Consider TTL per entry (optional but recommended) to prevent stale data. Implementation options: - Small internal LRU (map + doubly-linked list via `container/list`) - Or use a well-known Go LRU library (only if adding a dependency is acceptable). 3) **Periodic cleanup** Add a periodic cleanup mechanism: - A background goroutine with a `time.Ticker` (e.g. every N minutes) that prunes expired entries. - Ensure it stops cleanly on shutdown (context cancellation / stop channel). # Acceptance criteria - ✅ `go test -race ./...` shows **no data races** under load. - ✅ Cache cannot exceed `maxEntries` (validated by unit tests). - ✅ Eviction behaves like LRU (unit tests: access updates recency, insert triggers expected eviction). - ✅ Periodic cleanup removes expired entries (unit test with fake clock or controllable time source if possible). - ✅ No goroutine leaks (cleanup ticker stops on shutdown). Notes / hints - If LRU is implemented, operations typically require a lock anyway → `sync.RWMutex` + explicit LRU structure is straightforward. - If the cache is read-heavy, consider `RLock` for reads, but note that LRU “touch” on read requires upgrading to write; simplest approach is just a single `Lock` for Get/Set if performance is not critical. Tasks checklist - [ ] Identify cache structure in `policy.go` (around line 14) and all access points. - [ ] Add locking around cache operations (or refactor to a dedicated cache type). - [ ] Implement bounded LRU (`maxEntries`) and eviction. - [ ] Add TTL metadata and periodic cleanup. - [ ] Add tests + `run -race`.

peio added this to the mailcloak project 2026-01-26 00:27:09 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: peio/mailcloak#12