Repository navigation
Conversation
The disk state manager only reads a state from disk when it is not in memory, which in practice means the backend restarted, as on a hot reload. The saved state still held the values of cached vars, so a reload that only changed a getter kept showing the old value. Drop the cached computed var values of states read from disk and keep their fields. States served from memory keep their cache.
Contributor
[Medium impact] The PR appears safe to merge, with the previous finding fully addressed. SummaryThe PR clears cached computed values when loading saved states from disk, while keeping state fields and in-memory caches.
Reviews (2) · Last reviewed commit: "test: add a docstring to the subprocess ..." · Reviewed by Greptile |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Type of change
Changes To Core Features:
Description
reflex runkeeps states on disk so they survive a hot reload, and the saved state includes the values of cached vars. When a reload only changes a getter, the schema still matches, so the old value comes back and the page keeps showing it.StateManagerDiskonly reads a state from disk when it isn't in memory, which in practice means the backend just restarted. It now drops the cached computed var values of the states it reads from disk and keeps their fields. States served from memory keep their cache, and_deserializeis unchanged, so Redis, which reads states on every event, caches as before. That also means your probe in the issue, which calls_deserializedirectly, still prints the cached value.Tests (both fail without the fix, showing the old two-row value):
test_states_from_disk_recompute_cached_vars: a state and a substate saved by one manager are read by a new one. Fields are kept, cached vars are recomputed, and a read from memory still uses the cache.test_hot_reload_recomputes_cached_vars: the issue's app in two real processes, editing only the getter in between.I also tried the real dev server with the issue's app. After editing the getter, main still showed 2 rows after the reload and after a full page refresh, and this branch shows 1 row. A counter I had incremented kept its value both times. I didn't add a browser test because AppHarness runs the backend in-process, so it can't reload into a new process.
This doesn't cover Redis in dev mode, where every event reads the state from Redis. Telling a reload apart there would need something extra, like recording which process saved the state. Happy to add that here or in a follow-up if you want it.
closes #3989