fix(components): use identity equality for ComponentOwned lifecycle objects (#6776) - #6777
Open
chiliec wants to merge 1 commit into
Open
fix(components): use identity equality for ComponentOwned lifecycle objects (#6776)#6777chiliec wants to merge 1 commit into
chiliec wants to merge 1 commit into
Conversation
…bjects ComponentOwned and its subclasses (Hook, ObservableSubscription) are lifecycle objects tracked in lists and matched with `in`/`list.remove`. They are dataclasses whose only declared fields are InitVars, so the generated __eq__ compares no fields and every instance compares equal. In Component._detach_observable_subscription, `subscription in self._state.observable_subscriptions` then matches a different subscription and `.remove()` deletes the wrong one, leaking the subscription that should have been detached. Set eq=False on ComponentOwned, Hook and ObservableSubscription so they fall back to identity equality/hash. Add regression tests. Closes flet-dev#6776.
|
|
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.
What
Closes #6776.
ComponentOwnedand its subclassesHookandObservableSubscriptionare lifecycle objects tracked in lists and matched by identity (in,list.remove). They are dataclasses whose only declared fields areInitVars (owner,observable), so the auto-generated__eq__compares no fields — making every instance compare equal to every other.In
Component._detach_observable_subscription:subscription in ...matches a different subscription, and.remove()deletes the wrong one — so the subscription that should have been detached is leaked (the memory/subscription leak in the issue). The same collision affectsHook, which is stored per render slot and compared by identity.Fix
Set
eq=FalseonComponentOwned,Hook, andObservableSubscriptionso they fall back toobject's identity-based__eq__/__hash__. Each subclass is decorated with@dataclass, which regenerates__eq__, so the flag is applied to all three.Reproduction (before the fix)
After the fix all three return the correct result (
False,False, removess2).Tests
Added
tests/test_component_owned_identity.py(3 cases):ComponentOwned,Hook, andObservableSubscriptioneach use identity equality and behave correctly underin/list.remove.Verified RED→GREEN: reverting the three classes to plain
@dataclassfails all three new tests withassert ObservableSubscription() != ObservableSubscription(); witheq=Falsethey pass.Validation (real results, Python 3.11, pytest)
tests/test_component_owned_identity.py: 3/3 passtest_observable.py,test_component_renderer.py,test_component_diff.py,test_component_effects.py: all pass (16/16 together with the new file) — no regressions.(Some unrelated test modules fail to collect in my environment due to a missing optional
msgpackdependency; this happens identically on a cleanmaincheckout and is unrelated to this change.)Summary by Sourcery
Use identity equality for component-owned lifecycle objects to prevent incorrect list operations and leaked observable subscriptions.
Bug Fixes:
Tests: