Synchronize Handler shared state (H-10) #107

Merged

qwc merged 1 commit from fix/handler-state-race into main

2026-05-18 20:40:29 +02:00

qwc commented

2026-05-18 20:35:54 +02:00

Owner

Summary

Workstream item #12. Audit finding H-10 (High) — data race on shared mutable Handler state.

reindexRunning, reindexProgress, latestTagsCache, latestTagsCacheTime lived as bare fields on Handler and were accessed concurrently from HTTP goroutines and from the worker goroutine started in handleAdminReindex. Real data race in prod, not hypothetical.

Changes

New internal/handler/state.go with two typed structs, each holding its own sync.Mutex:
- reindexState — tryStart (atomic check-and-set, closes the race where two admins could each see running=false and spawn parallel workers), setProgress, finish, snapshot (returns both fields under one lock so the rendered page never shows running=false, progress="3/10 ...").
- latestTagsCache — get(now), set(now, entries), invalidate. TTL constant moved next to the type.
Four Handler fields replaced by two embedded structs.
All read/write sites updated (search.go getter + reindex handler, admin.go reindex render path, the six invalidateLatestTagsCache callers continue working through the forwarder method).
Two new concurrency tests (state_test.go) deliberately hammer both state types from many goroutines so go test -race catches any future refactor that drops the lock.

Test plan

go test ./... passes
go test -race ./... clean across the whole suite (211s for the handler package)
New concurrency tests pass under -race
Manual: trigger two admin reindexes in quick succession, confirm the second one shows "Reindex is already running" (it should — tryStart returns false)

🤖 Generated with Claude Code

## Summary Workstream item #12. Audit finding **H-10 (High)** — data race on shared mutable Handler state. `reindexRunning`, `reindexProgress`, `latestTagsCache`, `latestTagsCacheTime` lived as bare fields on `Handler` and were accessed concurrently from HTTP goroutines and from the worker goroutine started in `handleAdminReindex`. Real data race in prod, not hypothetical. ## Changes - New `internal/handler/state.go` with two typed structs, each holding its own `sync.Mutex`: - **`reindexState`** — `tryStart` (atomic check-and-set, closes the race where two admins could each see `running=false` and spawn parallel workers), `setProgress`, `finish`, `snapshot` (returns both fields under one lock so the rendered page never shows `running=false, progress="3/10 ..."`). - **`latestTagsCache`** — `get(now)`, `set(now, entries)`, `invalidate`. TTL constant moved next to the type. - Four `Handler` fields replaced by two embedded structs. - All read/write sites updated (`search.go` getter + reindex handler, `admin.go` reindex render path, the six `invalidateLatestTagsCache` callers continue working through the forwarder method). - Two new concurrency tests (`state_test.go`) deliberately hammer both state types from many goroutines so `go test -race` catches any future refactor that drops the lock. ## Test plan - [x] `go test ./...` passes - [x] `go test -race ./...` clean across the whole suite (211s for the handler package) - [x] New concurrency tests pass under `-race` - [ ] Manual: trigger two admin reindexes in quick succession, confirm the second one shows "Reindex is already running" (it should — `tryStart` returns false) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-05-18 20:35:54 +02:00

Synchronize Handler shared state with explicit mutex types (H-10)

CI / build (pull_request) Successful in 56s

Details

CI / docker (pull_request) Has been skipped

Details

CI / test (pull_request) Successful in 1m17s

Details

d82a3cefce

Audit finding H-10. reindexRunning, reindexProgress, latestTagsCache,
and latestTagsCacheTime lived as bare fields on Handler. They were
read and written from concurrent HTTP goroutines and from the worker
goroutine started in handleAdminReindex — a real data race in production,
not a hypothetical one.

Replace the four fields with two typed structs (internal/handler/state.go):

  reindexState   — sync.Mutex + running + progress
    methods: tryStart, setProgress, finish, snapshot

  latestTagsCache — sync.Mutex + entries map + stored time
    methods: get, set, invalidate

Update every read/write site:
  - handleAdminReindex now claims the slot via reindex.tryStart
    (atomically check-and-set) instead of read-then-write, closing the
    race where two admins could each see running=false and spawn
    parallel workers. The worker's defer calls reindex.finish.
  - handleAdminProjects reads both reindex fields atomically via
    snapshot, so the rendered page never shows a torn state like
    (running=false, progress="3/10 ...").
  - getLatestVersionTags / invalidateLatestTagsCache go through the
    cache's get/set/invalidate methods.

Two new concurrency tests (state_test.go) deliberately hammer both
state types from many goroutines so `go test -race` catches any future
refactor that drops the lock — the existing tests don't exercise this
shape and so wouldn't have flagged H-10 either.

`go test -race ./...` clean across the whole suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qwc merged commit e7d9621fd5 into main

2026-05-18 20:40:29 +02:00

qwc deleted branch fix/handler-state-race

2026-05-18 20:40:29 +02:00

qwc referenced this pull request from a commit

2026-05-18 20:40:30 +02:00

Merge pull request 'Synchronize Handler shared state (H-10)' (#107) from fix/handler-state-race into main

qwc referenced this pull request from a commit

2026-07-13 13:19:10 +02:00

Merge pull request 'Synchronize Handler shared state (H-10)' (#107) from fix/handler-state-race into main