Fix Memory Leak for Media Element in Windows#3115
Fix Memory Leak for Media Element in Windows#3115ne0rrmatrix wants to merge 5 commits intoCommunityToolkit:mainfrom
Conversation
…r class that is released when dispose is called. I see no need for two events that handle the same event. I should never have had two to start with. This fixes that.
TheCodeTraveler
requested review from
Copilot
and removed request for
TheCodeTraveler
There was a problem hiding this comment.
Pull request overview
This PR fixes a memory leak in MediaElement on Windows by removing a duplicate event handler that was not being properly unsubscribed during disposal. The issue was caused by FullScreenButton_Click event handler in CustomTransportControls that maintained local state and updated the fullscreen button icon, but was never unsubscribed when the control was disposed.
Changes:
- Removed the duplicate
FullScreenButton_Clickevent handler andisFullScreenfield fromCustomTransportControls - Moved icon state updates directly into the existing
OnFullScreenButtonClickhandler inMauiMediaElement, which is properly unsubscribed during disposal - Simplified state management by using the actual window presenter state (
appWindow.Presenter.Kind) instead of maintaining a local boolean
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/CommunityToolkit.Maui.MediaElement/Views/MauiMediaElement.windows.cs | Added icon updates to the existing OnFullScreenButtonClick handler based on actual window state |
| src/CommunityToolkit.Maui.MediaElement/Primitives/CustomTransportControls.windows.cs | Removed duplicate event handler and state field that were causing the memory leak |
|
Thank you for the quick work on this 😄 Appreciate it and the honesty 😆 Will this fix be ported to 6.x? Our app is currently still on .NET 9 |
We only support the current version of Dotnet which is dotnet 10. We are all volunteers and supporting multiple release branches is not supported at this time. |
Description of Change
There was a button with an event attached to it that was not properly disposed of when class was being disposed. I have removed the event, it was an unneeded duplicate and moved what it did into an existing event that is unsubscribed in disposal for media element. Thus I have removed an extra event and this should fix the issue as described.
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
This is the direct result of failing to unsubscribe from an event in the disposal method for the class. I have moved the behavior in event to another class which has a duplicate event handler. I do not know why I was using two different events. I switched to just using one and removed the offending event. I see no reason for having two events for same event. I can't explain why I would have done that.