Track background jobs; wire session + rate-limit cleanup (M-12, M-13) #112

Merged

qwc merged 1 commit from fix/background-jobs-lifecycle into main

2026-05-18 22:48:22 +02:00

qwc commented

2026-05-18 22:45:09 +02:00

Owner

Summary

Workstream item #17. Two related audit findings.

M-13 — fire-and-forget goroutines

Three sites started goroutines the process abandoned on shutdown:

upload.go + api.go: searchIndex.IndexVersion (post-upload reindex)
upload.go + api.go: enforceRetentionPolicy(context.Background())
search.go: handleAdminReindex's worker goroutine

enforceRetentionPolicy was called with context.Background(), so even cancelling the retention worker's context didn't stop a per-upload retention scan.

New internal/handler/jobs.go adds:

jobs struct holding a cancellable context + sync.WaitGroup
h.runJob(fn) registers fn under the WaitGroup; fn receives the shared cancellable context
h.StopBackgroundJobs() cancels and waits

All three fire-and-forget sites migrated to runJob. enforceRetentionPolicy now runs under the cancellable jobs context — a long retention scan exits promptly on shutdown.

M-12 — unused cleanup paths

SessionStore.DeleteExpired and RateLimiter.Cleanup existed but were only called from tests. Sessions accumulated indefinitely; the rate-limit map grew linearly with distinct client IPs since boot.

New StartBackgroundWorkers method spawns three loops under runJob:

StartRetentionWorker (existing, now wrapped)
sessionCleanupLoop — DeleteExpired every hour, immediate first run
rateLimitCleanupLoop — Cleanup every 10 minutes

All three exit on jobs context cancel.

main.go

Replaces the bespoke retentionCtx/retentionCancel + bare go h.StartRetentionWorker(...) with one h.StartBackgroundWorkers().
After server.Shutdown returns, calls h.StopBackgroundJobs() so in-flight indexing / retention sweeps / cleanup ticks drain before the process exits.

Tests

internal/handler/jobs_test.go:

TestRunJobAwaitsCompletion — StopBackgroundJobs blocks until the job exits.
TestRunJobCancelsContextOnStop — jobs that watch ctx.Done exit promptly when stop fires.
TestStopBackgroundJobsIsSafeWhenIdle — idle Stop returns immediately.

All verified under -race.

Test plan

go test ./... and -race pass
Manual: trigger an upload, then SIGTERM the server. The log should show "draining background jobs" and the process should exit only after indexing finishes (visible as the next-line log).
Manual: after the server has been running for a while, check sessions table — expired rows are gone (visible after the first hour). Rate-limit map growth — harder to observe directly, but Login attempts from many distinct IPs should eventually plateau.

🤖 Generated with Claude Code

## Summary Workstream item #17. Two related audit findings. ### M-13 — fire-and-forget goroutines Three sites started goroutines the process abandoned on shutdown: - `upload.go` + `api.go`: `searchIndex.IndexVersion` (post-upload reindex) - `upload.go` + `api.go`: `enforceRetentionPolicy(context.Background())` - `search.go`: `handleAdminReindex`'s worker goroutine `enforceRetentionPolicy` was called with `context.Background()`, so even cancelling the retention worker's context didn't stop a per-upload retention scan. New `internal/handler/jobs.go` adds: - `jobs` struct holding a cancellable context + `sync.WaitGroup` - `h.runJob(fn)` registers `fn` under the WaitGroup; `fn` receives the shared cancellable context - `h.StopBackgroundJobs()` cancels and waits All three fire-and-forget sites migrated to `runJob`. `enforceRetentionPolicy` now runs under the cancellable jobs context — a long retention scan exits promptly on shutdown. ### M-12 — unused cleanup paths `SessionStore.DeleteExpired` and `RateLimiter.Cleanup` existed but were only called from tests. Sessions accumulated indefinitely; the rate-limit map grew linearly with distinct client IPs since boot. New `StartBackgroundWorkers` method spawns three loops under `runJob`: - `StartRetentionWorker` (existing, now wrapped) - `sessionCleanupLoop` — `DeleteExpired` every hour, immediate first run - `rateLimitCleanupLoop` — `Cleanup` every 10 minutes All three exit on jobs context cancel. ### main.go - Replaces the bespoke `retentionCtx`/`retentionCancel` + bare `go h.StartRetentionWorker(...)` with one `h.StartBackgroundWorkers()`. - After `server.Shutdown` returns, calls `h.StopBackgroundJobs()` so in-flight indexing / retention sweeps / cleanup ticks drain before the process exits. ## Tests `internal/handler/jobs_test.go`: - `TestRunJobAwaitsCompletion` — `StopBackgroundJobs` blocks until the job exits. - `TestRunJobCancelsContextOnStop` — jobs that watch `ctx.Done` exit promptly when stop fires. - `TestStopBackgroundJobsIsSafeWhenIdle` — idle Stop returns immediately. All verified under `-race`. ## Test plan - [x] `go test ./...` and `-race` pass - [ ] Manual: trigger an upload, then SIGTERM the server. The log should show "draining background jobs" and the process should exit only after indexing finishes (visible as the next-line log). - [ ] Manual: after the server has been running for a while, check `sessions` table — expired rows are gone (visible after the first hour). Rate-limit map growth — harder to observe directly, but Login attempts from many distinct IPs should eventually plateau. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-05-18 22:45:10 +02:00

Track background jobs; wire session + rate-limit cleanup (M-12, M-13)

CI / test (pull_request) Successful in 1m25s

Details

CI / build (pull_request) Successful in 59s

Details

CI / docker (pull_request) Has been skipped

Details

587e065127

Audit findings M-12 and M-13: background goroutines and unused cleanup
paths were both lifecycle gaps that this PR closes together.

M-13 — fire-and-forget goroutines:
  Three sites started goroutines that the process abandoned on shutdown:
    - upload.go and api.go: searchIndex.IndexVersion (post-upload reindex)
    - upload.go and api.go: enforceRetentionPolicy(context.Background())
    - search.go: handleAdminReindex's worker goroutine
  enforceRetentionPolicy explicitly used context.Background, so even
  cancelling the retention worker's context didn't stop a retention scan
  spawned from an upload mid-flight.

  New internal/handler/jobs.go adds:
    - jobs struct (ctx + cancel + WaitGroup)
    - Handler.runJob(fn) registers fn under the WaitGroup; fn receives
      the cancellable jobs context
    - Handler.StopBackgroundJobs() cancels and waits

  Every fire-and-forget goroutine migrated to runJob. enforceRetentionPolicy
  now gets the cancellable jobs context so a long retention scan exits
  promptly on shutdown.

M-12 — unused cleanup paths:
  SessionStore.DeleteExpired and RateLimiter.Cleanup existed but were
  only called from tests. Sessions accumulated indefinitely, the
  rate-limit map grew linearly with distinct client IPs since boot.

  New StartBackgroundWorkers method spawns three loops under runJob:
    - StartRetentionWorker (already existed, just re-wrapped)
    - sessionCleanupLoop: DeleteExpired every hour, immediate first run
    - rateLimitCleanupLoop: Cleanup every 10 minutes
  All three exit on jobs context cancel.

main.go:
  - Replaces the bespoke retentionCtx/retentionCancel + bare
    `go h.StartRetentionWorker(...)` with one h.StartBackgroundWorkers().
  - After server.Shutdown returns, calls h.StopBackgroundJobs() so
    in-flight indexing / retention sweeps / cleanup ticks drain before
    the process exits.

Three regression tests (jobs_test.go) cover runJob's WaitGroup behavior,
context cancellation propagation, and idle-Stop being a no-op. All
verified under -race.

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

qwc merged commit 390ab8aa85 into main

2026-05-18 22:48:22 +02:00

qwc deleted branch fix/background-jobs-lifecycle

2026-05-18 22:48:22 +02:00

qwc referenced this pull request from a commit

2026-05-18 22:48:23 +02:00

Merge pull request 'Track background jobs; wire session + rate-limit cleanup (M-12, M-13)' (#112) from fix/background-jobs-lifecycle into main

qwc referenced this pull request from a commit

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

Merge pull request 'Track background jobs; wire session + rate-limit cleanup (M-12, M-13)' (#112) from fix/background-jobs-lifecycle into main