Skip to content

fix: replace unsafe Arc mutation in PUT /api/budget with RwLock#782

Open
rager306 wants to merge 1 commit intoRightNow-AI:mainfrom
rager306:fix/safe-budget-mutation
Open

fix: replace unsafe Arc mutation in PUT /api/budget with RwLock#782
rager306 wants to merge 1 commit intoRightNow-AI:mainfrom
rager306:fix/safe-budget-mutation

Conversation

@rager306
Copy link

Summary

PUT /api/budget currently casts &Arc<AppState> to *mut KernelConfig and mutates budget fields through a raw pointer:

// routes.rs ~line 5282
let config_ptr = &state.kernel.config as *const KernelConfig
    as *mut KernelConfig;
unsafe {
    (*config_ptr).budget.max_hourly_usd = v;
    // ...
}

This is unsound: AppState is wrapped in Arc and shared across all Tokio worker threads. Two concurrent PUT /api/budget requests produce a data race on the same memory. The Rust compiler gives no guarantees for this pattern — the optimizer can reorder or elide the stores.

Fix

Add pub budget_config: Arc<tokio::sync::RwLock<BudgetConfig>> to AppState, initialized from kernel.config.budget at startup. Readers use .read().await, writer uses .write().await. Zero unsafe code in the budget update path.

Testing

  • cargo build -p openfang-api passes
  • No new unsafe blocks introduced
  • Concurrent budget updates are now serialized via RwLock

PUT /api/budget casts &Arc<AppState> to *mut KernelConfig and mutates
the budget fields through a raw pointer. This is unsound: AppState is
shared across Tokio worker threads, so two concurrent PUT /api/budget
requests cause a data race on the same memory location.

Replace with Arc<tokio::sync::RwLock<BudgetConfig>> stored on AppState,
initialized from kernel.config.budget at startup. All readers use
.read().await and all writers use .write().await. No unsafe code remains
in the budget update path.

Fixes: data race / undefined behaviour under concurrent budget updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant