Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: m_lastSeenParent should be a weak reference (would you accept a PR?) #799

Open
benstevens48 opened this issue Nov 30, 2020 · 1 comment

Comments

@benstevens48
Copy link

Following on from this issue #658 and by looking at the code, I can see that Win2D controls hold a strong reference to their parent (called m_lastSeenParent). This is to enable RemoveFromVisualTree to work correctly.

I propose changing m_lastSeenParent to a WeakRef. Would you accept a PR for this?

Reasoning
The current behavior causes a number of problems. As mentioned in the other issue, even unsubscribing all events from the control will not prevent a memory leak because there is this strong reference to the parent remaining. Also, this means that RemoveFromVisualTree must even be called in C++ apps. Both of these are in contradiction to the docs here.

Also, it's not always feasible to have a Dispose-type method on the containing control - in my case I am using CanvasVirtualControl in a reusable component, and requiring a Dispose on this would require a Dispose on all possible parent controls. What I want to do is unsubscribe events on Unloaded and subscribe them on Loaded, but this won't prevent a memory leak due to the strong parent ref held by the CanvasVirtualControl. (Also, holding a strong parent ref after unloading from the visual tree is the exact opposite of what the framework controls do. It makes more sense to me that the child should hold no strong refs to the parent, rather than trying to do it the other way around).

@benstevens48 benstevens48 changed the title Suggestion: m_lastSeenParent should be weak reference (would you accept a PR?) Suggestion: m_lastSeenParent should be a weak reference (would you accept a PR?) Nov 30, 2020
@benstevens48
Copy link
Author

I created a pull request #801 for this.

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

No branches or pull requests

1 participant