Skip to content

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

Draft
adhami3310 wants to merge 2 commits into
khaleel/background-unlocked-trailing-cleanfrom
khaleel/rfc-locked-root
Draft

RFC(events): lock ownership as a capability for delta flushes#6921
adhami3310 wants to merge 2 commits into
khaleel/background-unlocked-trailing-cleanfrom
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 (7b7e5ce) with khaleel/background-unlocked-trailing-clean (3d4975b)

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

This draft RFC introduces a LockedRoot capability so delta flushes explicitly require a claim of token-lock ownership.

  • Adds guarded LockedRoot construction through mint_locked_root.
  • Updates foreground, rehydration, background-yield, and compatibility-flush paths to pass the capability.
  • Adds tests rejecting bare state roots 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 documented capability-minting function.
packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py Requires LockedRoot for delta flushing and mints it at audited lock-holding call sites.
tests/units/reflex_base/event/processor/test_base_state_processor.py Updates lock-proof expectations and tests rejection of bare roots and direct wrapper construction.

Reviews (5): 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 from 7d8434b to 99f8127 Compare August 21, 2026 18:04
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 99f8127 to 7b7e5ce Compare August 21, 2026 18:29
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