State.reset() should not reset inherited backend vars#6365
Conversation
`self.backend_vars` contains backend vars from the current state instance and its parent states. When performing the reset, we should never touch vars that belong to the parent(s).
Ensure that a State instance's _backend_vars only copies backend vars that were defined in the given substate, and does not also copy inherited backend vars that are managed by the parent. Ensures that assigning an inherited backend var through a substate marks the owner as touched and not the substate it was assigned through.
`self._backend_vars` only contains vars owned by the given instance, so mark these dirty instead of _all_ backend vars that are owned or inherited.
assert that the current instance's backend vars have been reset to non-inherited backend vars.
assert that the newly reset values equal the default values for non-inherited vars
Greptile SummaryThis PR fixes a correctness bug where Confidence Score: 5/5Safe to merge; the fix is well-scoped and all remaining findings are P2 (test robustness suggestion only). The core logic change is correct and consistent across all affected paths (init, reset, _get_was_touched, _patch_state). Three unit tests directly verify the new invariants, and an integration test exercises the reported end-to-end scenario. The only finding is a P2 concern about poll ordering in the integration test. tests/integration/test_linked_state.py — poll ordering after reset could pass on stale DOM content Important Files Changed
Reviews (1): Last reviewed commit: "tighter assertion in test_component_stat..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
explicitly reset the value that we're checking to blank before asserting that it is not blank to avoid a case where a bad value overwrites the checked value _after_ it was checked.
Internally, SharedStateBaseInternal keeps track of which shared lock tokens it is currently holding. If the user requests patching in a SharedState, but doesn't currently hold the lock, then it will request the lock for the linked token, store it in `_held_locks`, then load the state and patch it into the tree. This is problematic when async computed vars are requesting the same SharedState instance, because resolving each async var value runs in its own task, so each var function races with the others to complete. Since these all depend on acquiring the same lock, without an asyncio.Lock protecting the critical `_held_locks` structure, each var will attempt to acquire the redis lock to the exclusion of the other var tasks. With the new `_held_locks_lock`, the internal machinery serializes the acquisition of the linked token lock so that subsequent access can properly reuse a single acquired lock, rather than each trying to acquire it and hold it for the duration of the event being processed.
2 participants
self.backend_varscontains backend vars from the current state instance and its parent states. When performing the reset, we should never touch vars that belong to the parent(s).Ensure that a State instance's _backend_vars only copies backend vars that were defined in the given substate, and does not also copy inherited backend vars that are managed by the parent. Ensures that assigning an inherited backend var through a substate marks the owner as touched and not the substate it was assigned through.