Repository navigation
DO NOT MERGE - Disconnect shared state - #6942
benedikt-bartscher wants to merge 13 commits into
Conversation
…ce period Resolves the TODO in _do_update_other_tokens: tokens of clients that disconnected and did not reconnect within REFLEX_SHARED_STATE_DISCONNECT_GRACE (default 30s, 0 disables) are removed from the _linked_from subscriber sets of all shared states the client was linked to, so shared mutations stop fanning out modify_state churn for them. The new SharedState._on_subscriber_disconnected hook lets shared states clean up per-client data (e.g. presence bookkeeping). Safety: a reconnect within the grace (any instance, checked via is_token_connected) is a no-op; a client reaped too eagerly re-subscribes automatically on its next event through _internal_patch_linked_state. A new disconnect for the same token restarts the grace. Only the shared subscriber sets are touched, never the client's own _reflex_internal_links.
 [Medium impact] The change affects shared-state subscriptions across apps that use linked shared states. The PR should not merge until disconnect cleanup can still find subscriptions after the client's saved state expires. Findings
SummaryAdds delayed cleanup for clients that disconnect from shared states.
Reviews (9) · Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance1 warning
Comparing 
|
…in/disconnect-shared-state # Conflicts: # reflex/app.py # reflex/istate/shared.py # reflex/utils/token_manager.py # tests/units/istate/test_shared.py
| root_state = await app.state_manager.get_state( | ||
| BaseStateToken(ident=token, cls=app._state) | ||
| ) | ||
| if not isinstance(root_state, State): | ||
| return | ||
| links = dict(root_state._reflex_internal_links or {}) |
There was a problem hiding this comment.
_reap_disconnected_client relies on the client's saved state still existing after the grace period. Redis expires that state based on its last write, not when the socket disconnects. If a client disconnects shortly before its state expires, get_state creates a fresh state with no saved _reflex_internal_links. Cleanup then skips every subscription, leaving the client in an active shared state's _linked_from and never calling _on_subscriber_disconnected.
Keep the link information available until disconnect cleanup finishes.
…halfway Recheck the connection under the shared-state lock before unlinking, and let a reap past its grace period finish instead of cancelling it on the next disconnect.
DISCONNECT_REAP_TASKS now keeps the latest reap per token until it finishes. Under each shared state's lock, a reap rechecks that the client is still gone and that no newer reap replaced it; only then does it join STARTED_REAP_TASKS and stop being cancellable, so a reap superseded while waiting for the lock leaves the unlink to the new grace period.
…tate on_disconnect now cancels the client's page-load chains (only for its current local socket) and then schedules the shared-state reap unconditionally, as before.
based on #6934