Skip to content

Fix race condition in state store cache updates #1753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 8, 2025

Conversation

sanity
Copy link
Collaborator

@sanity sanity commented Aug 7, 2025

Summary

  • Fixed a race condition in the StateStore where the memory cache was updated after the persistent store
  • This could cause GET requests to read stale cached data during the update window
  • Now updates memory cache first, then persistent store

Details

When multiple concurrent operations access the state store, there was a race condition where:

  1. Thread A starts updating state (writes to persistent store first)
  2. Thread B reads state (gets stale data from cache while persistent store is being updated)
  3. Thread A finishes updating (cache now updated, but Thread B already has stale data)

This fix ensures the memory cache is updated first, preventing other threads from reading stale data during the update window.

Test plan

While this fix didn't resolve the specific River chat test failure we were investigating, it does fix a legitimate race condition that could cause issues in high-concurrency scenarios. The change is a defensive improvement to prevent potential bugs.

  • Cargo tests pass
  • Code formatted with cargo fmt
  • Clippy checks pass

🤖 Generated with Claude Code

…hange (#1734)

Co-authored-by: Claude <noreply@anthropic.com>
@CLAassistant
Copy link

CLAassistant commented Aug 7, 2025

CLA assistant check
All committers have signed the CLA.

…ition

When multiple concurrent operations access the state store, there was a race
condition where GET requests could read stale cached data while the persistent
store was being updated. This fix ensures the memory cache is updated first,
preventing other threads from reading stale data during the update window.

This change affects both update() and store() methods in StateStore.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sanity sanity force-pushed the fix-state-store-cache-race-condition branch from e4103db to d410901 Compare August 7, 2025 18:33
sanity and others added 2 commits August 8, 2025 00:20
Co-authored-by: Ian Clarke <ian@freenet.org>
Co-authored-by: Claude <noreply@anthropic.com>
@sanity sanity force-pushed the fix-state-store-cache-race-condition branch from e1db586 to 74cb02e Compare August 8, 2025 05:07
@sanity
Copy link
Collaborator Author

sanity commented Aug 8, 2025

Rebased onto latest main after #1754 (timeout increases) merged. Re-running CI to pick up test stability improvements.

@sanity sanity enabled auto-merge (squash) August 8, 2025 16:31
@sanity sanity merged commit 3f30d53 into main Aug 8, 2025
6 checks passed
@sanity sanity deleted the fix-state-store-cache-race-condition branch August 8, 2025 16:40
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.

2 participants