-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[iOS] Fix: memory leak when Navigation.RemovePage is used #5695
Conversation
The leak occurs in a MasterDetailPage.Detail = NavigationPage(ContentPage) scenario if the ContentPage has ToolBarItems. (ToolBarItems create a strong dependency therefore the ViewController of the NavigationPage must be explicitly disposed)
@kicsiede Just a suggestion, adding an unit test for this would be great... It's really awesome you were able to find and track this down, memory leaks are 😨 |
Hey do you think you can you add a UITest ?! thanks |
@rmarinho please check |
namespace Xamarin.Forms.Controls.Issues | ||
{ | ||
[Preserve(AllMembers = true)] | ||
[Issue(IssueTracker.Github, 5555, "Memory leak when SwitchCell or EntryCell", PlatformAffected.iOS)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy/paste detected =)
I'll think about something more universal for any controls when checking for memory leaks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I was gonna say we had a test for most of the renderers where we pushed a page with a control, and make sure it was collected, I wonder if we can do that in the dynamic gallery test you created Pavel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The leak occurs in a MasterDetailPage.Detail = NavigationPage(ContentPage) scenario if the ContentPage has ToolBarItems. (ToolBarItems create a strong dependency therefore the ViewController of the NavigationPage must be explicitly disposed)
Description of Change
disposing viewcontroller in RemovePage.
Platforms Affected
Testing Procedure
PR Checklist