Skip to content

RFC(events): lock ownership as a capability for delta flushes - #6921

Draft
adhami3310 wants to merge 2 commits into
mainfrom
khaleel/rfc-locked-root
Draft

RFC(events): lock ownership as a capability for delta flushes#6921
adhami3310 wants to merge 2 commits into
mainfrom
khaleel/rfc-locked-root

Conversation

@adhami3310

@adhami3310 adhami3310 commented Aug 20, 2026

Copy link
Copy Markdown
Member

RFC / draft for discussion — one of two competing shapes for hardening the delta-flush path after #6920. Not intended to merge as-is.

#6920 fixed a lost-update race where a background event flushed (snapshot, resolve, emit, _clean()) a shared state tree after dropping the token lock. The fix gates the one bad caller. This RFC makes the class unrepresentable instead: delta flushes require proof of lock ownership.

  • mint_locked_root(root) in reflex.istate.manager is the single, greppable, documented claim of "I hold the token lock for this root". LockedRoot cannot be constructed any other way.
  • chain_updates takes LockedRoot | None and raises TypeError on a bare BaseState.
  • The lock-holding call sites (foreground branch, _rehydrate, the compatibility flush) mint inside their modify_state blocks. The background-yield gate keeps its _is_mutable() logic but now mints at that one audited spot, which is where its justification already lived in a comment.

Enforcement is convention plus one constructor guard — as strong as Python gets — but the payoff is that "who may flush" went from call-site history to a reviewable value, and any future unlocked flush is a loud TypeError in development rather than a silent lost update in production.

Not converted here (follow-up if the shape is liked): app.modify_state, StateProxy.__aexit__, and State.hydrate do the same four-step flush inline and could take the same capability — ideally via one shared DeltaFlush helper replacing the four copies.

Competing RFC: #6922 — instead of proving the lock, it makes the flush itself safe under concurrency (resolution-dirt ledger + selective clean), which additionally un-reverts the atomicity hardening that test_linked_state forced out of #6920. The two compose, but each is sufficient reviewed alone.

Tests: the existing #6920 suite passes unchanged; test_chain_updates_refuses_a_bare_root_state covers the enforcement; tests/integration/test_linked_state.py passes (the SharedState oracle).

@codspeed-hq

codspeed-hq Bot commented Aug 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing khaleel/rfc-locked-root (3aa9c90) with main (12d29c7)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The RFC introduces a guarded LockedRoot capability so delta flushing requires an explicit claim of token-lock ownership.

  • Adds the guarded capability and its minting function to the state manager.
  • Updates foreground, hydration, background-proxy, and compatibility flush paths to pass the capability.
  • Adds coverage rejecting bare root states and direct capability construction.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains.

No blocking failure remains.

Important Files Changed

Filename Overview
reflex/istate/manager/init.py Adds the guarded LockedRoot wrapper and the single documented minting function.
packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py Requires lock-ownership capabilities for delta flushes and updates all repository call sites consistently.
tests/units/reflex_base/event/processor/test_base_state_processor.py Updates lock-gating assertions and tests rejection of bare roots and direct wrapper construction.

Reviews (7): Last reviewed commit: "chore: trigger CI against main base" | Re-trigger Greptile

@adhami3310
adhami3310 changed the base branch from khaleel/background-unlocked-trailing-clean to main August 20, 2026 21:31
@adhami3310
adhami3310 changed the base branch from main to khaleel/background-unlocked-trailing-clean August 20, 2026 21:59
@adhami3310
adhami3310 force-pushed the khaleel/rfc-locked-root branch 2 times, most recently from 99f8127 to 7b7e5ce Compare August 21, 2026 18:29
Base automatically changed from khaleel/background-unlocked-trailing-clean to main August 25, 2026 17:48
@adhami3310
adhami3310 force-pushed the khaleel/rfc-locked-root branch from 7b7e5ce to f9b64fb Compare August 25, 2026 17:52
Delta work on a shared state tree is only safe while the token lock is
held, but nothing in the code records who holds it: chain_updates takes
a bare BaseState, and whether the caller is inside modify_state is call
site history. The lost-update bug fixed on this branch was exactly a
caller flushing a root it no longer owned.

Make ownership a value. mint_locked_root is the single audited claim of
the precondition, LockedRoot is the proof, and chain_updates refuses a
bare state with a TypeError. The proxy-yield gate keeps its logic but
now mints at the one place the justification lives; every other minting
site is inside the lock by construction.

Enforcement is by convention plus one greppable constructor, as strong
as Python allows. The other flush sites (app.modify_state, proxy exit,
hydrate) can adopt the same shape as follow-up.
@adhami3310
adhami3310 force-pushed the khaleel/rfc-locked-root branch from f9b64fb to 3aa9c90 Compare August 26, 2026 02:24
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