cmd/anubis: Fix potential decaymap race (#683)

Fixes a potential TOCTOU issue that would cause values to be spuriously
erased.

IIUC, the following interleaving of (*DecayMap).Get() and
(*DecayMap).Set() can cause an update to be erased:

  // thread A: Get("x")
  m.lock.RLock()
  value, ok := m.data["x"]
  m.lock.RUnlock()
  ...
  if time.Now().After(value.expiry) {
      // <wait for lock!>

  // thread B: Set("x", ...)
  m.lock.Lock()
  defer m.lock.Unlock()
  m.data["x"] = DecayMapEntry{ ... }

  // thread A continues its Get("x") after acquring the lock:
  m.lock.Lock()
  delete(m.data, "x") // Oops! Newer entry is deleted!
  m.lock.Unlock()

Realistically... I think it's probably a non-issue either way, because
the worst that can happen is that a cache entry is spuriously removed,
and it'll just get re-fetched.

1 files changed, 5 insertions, 1 deletions

diff --git a/cmd/anubis/decaymap.go b/cmd/anubis/decaymap.go
index bf55d2a..f97c4c6 100644
--- a/cmd/anubis/decaymap.go
+++ b/cmd/anubis/decaymap.go
@@ -37,7 +37,11 @@ func (m *DecayMap[K, V]) Get(key K) (V, bool) {
 
 	if time.Now().After(value.expiry) {
 		m.lock.Lock()
-		delete(m.data, key)
+		// Since previously reading m.data[key], the value may have been updated.
+		// Delete the entry only if the expiry time is still the same.
+		if m.data[key].expiry == value.expiry {
+			delete(m.data, key)
+		}
 		m.lock.Unlock()
 
 		return zilch[V](), false