refactor(api): make WebhookStore async-safe with tokio RwLock (closes #2)#6256
Conversation
The webhook store guarded its in-memory state with `std::sync::RwLock` and exposed synchronous `list`/`get`/`create`/`update`/`delete` methods called directly from async axum handlers. Holding a blocking lock (and running synchronous `std::fs` persistence) on the runtime threads can stall the executor under contention. Switch the guard to `tokio::sync::RwLock` and make the accessor methods `async`. Persistence now runs the file I/O on `spawn_blocking` so the atomic write never blocks the async runtime. `load` stays synchronous: it runs once at daemon boot / test setup, off the request path. Handlers are updated to `.await` the store calls. Where a handler held the `!Send` `ErrorTranslator` across the new await point, the translated messages are resolved up front and the translator dropped before the await (per CLAUDE.md), keeping the axum `Handler` bound satisfied. Closes librefang#2
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
LGTM. Reviewed: the std→tokio RwLock migration is correct and complete — all WebhookStore methods are now async, every one of the 6 call sites was updated, the !Send ErrorTranslator is resolved-and-dropped before each .await, disk I/O moved to spawn_blocking, and there's no deadlock or lock-ordering hazard. Dropping the unwrap_or_else(|e| e.into_inner()) poison recovery is correct since tokio::sync::RwLock doesn't poison. Real CI — Quality/clippy, all 4 Ubuntu test shards, Unit, Build, Security, Live Integration Smoke — is green.
The red detect-secrets scan check is a stale-base artifact: this branch predates #6262, which retired detect-secrets in favor of gitleaks, so the legacy baseline gate fails here. It is not a real secret. Clean to merge onto current (gitleaks) main.
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
LGTM — reviewed clean (tokio RwLock migration, all call sites updated, no deadlock/!Send-across-await); real Rust CI green; branch updated to current main.
2 participants
Problem
WebhookStoreguarded its in-memory state withstd::sync::RwLockand exposed synchronouslist/get/create/update/deletemethods that are called directly from async axum handlers.A blocking lock acquired on a runtime worker thread can stall the executor under contention, and the synchronous
std::fspersistence on the write path compounds it by blocking the thread during disk I/O.Fixes #2.
Change
webhook_store.rs: replacestd::sync::RwLockwithtokio::sync::RwLock;list/get/create/update/deleteare nowasync.persistruns the atomic file write ontokio::task::spawn_blockingso disk I/O never blocks the async runtime.loadstays synchronous on purpose — it is called once at daemon boot / test setup, off the async request path — with a doc-comment explaining why.routes/webhooks.rs: handlers.awaitthe store calls. Inget/create/update/delete, the!SendErrorTranslatorwas being held across the new await point (breaking the axumHandlerbound); resolve the translated messages up front and drop the translator before the await, matching the existing pattern intest_webhook(per CLAUDE.md).#[tokio::test]and.awaitthe async methods.tests/api_deny_unknown_fields_test.rs:.awaitthelist()call.Verification
cargo fmt -p librefang-apicargo check -p librefang-api --all-targets— cleancargo clippy -p librefang-api --all-targets -- -D warnings— cleancargo test -p librefang-api --lib webhook_store— 38 passedcargo test -p librefang-api --test api_deny_unknown_fields_test— 2 passed