Skip to content

Prevent leaking event hooks in our MauiEventHandler #4159

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 2, 2025
Merged

Prevent leaking event hooks in our MauiEventHandler #4159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
364b8a6
Always unhook events first to ensure any recalls due to collectionvie…
aritchie Apr 29, 2025
9b630b8
Update CHANGELOG.md
aritchie May 2, 2025
4f69fde
Merge branch 'main' into maui_mem_leak_fix
aritchie May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
### Fixes

- Work around iOS SHA1 bug ([#4143](https://github.com/getsentry/sentry-dotnet/pull/4143))
- Prevent Auto Breadcrumbs Event Binder from leaking and rebinding events ([#4159](https://github.com/getsentry/sentry-dotnet/pull/4159))
- Fixes build error when building .NET Framework applications using Sentry 5.6.0: `MSB4185 :The function "IsWindows" on type "System.OperatingSystem" is not available` ([#4160](https://github.com/getsentry/sentry-dotnet/pull/4160))

### Dependencies
Expand Down
141 changes: 70 additions & 71 deletions src/Sentry.Maui/Internal/MauiEventsBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public MauiEventsBinder(IHub hub, IOptions<SentryMauiOptions> options, IEnumerab

public void HandleApplicationEvents(Application application, bool bind = true)
{
// we always unbind first to ensure no previous hooks
UnbindApplication(application);

if (bind)
{
// Attach element events to all existing descendants (skip the application itself)
Expand Down Expand Up @@ -65,22 +68,23 @@ public void HandleApplicationEvents(Application application, bool bind = true)
// https://docs.microsoft.com/dotnet/maui/user-interface/system-theme-changes#react-to-theme-changes
application.RequestedThemeChanged += OnApplicationOnRequestedThemeChanged;
}
else
{
application.DescendantAdded -= OnApplicationOnDescendantAdded;
application.DescendantRemoved -= OnApplicationOnDescendantRemoved;
}

HandleElementEvents(application, bind: false);
private void UnbindApplication(Application application)
{
application.DescendantAdded -= OnApplicationOnDescendantAdded;
application.DescendantRemoved -= OnApplicationOnDescendantRemoved;

// Navigation events
application.PageAppearing -= OnApplicationOnPageAppearing;
application.PageDisappearing -= OnApplicationOnPageDisappearing;
application.ModalPushed -= OnApplicationOnModalPushed;
application.ModalPopped -= OnApplicationOnModalPopped;
HandleElementEvents(application, bind: false);

// Theme changed event
application.RequestedThemeChanged -= OnApplicationOnRequestedThemeChanged;
}
// Navigation events
application.PageAppearing -= OnApplicationOnPageAppearing;
application.PageDisappearing -= OnApplicationOnPageDisappearing;
application.ModalPushed -= OnApplicationOnModalPushed;
application.ModalPopped -= OnApplicationOnModalPopped;

// Theme changed event
application.RequestedThemeChanged -= OnApplicationOnRequestedThemeChanged;
}

internal void OnApplicationOnDescendantAdded(object? _, ElementEventArgs e)
Expand Down Expand Up @@ -169,6 +173,7 @@ internal void OnApplicationOnDescendantRemoved(object? _, ElementEventArgs e)

internal void HandleWindowEvents(Window window, bool bind = true)
{
UnhookWindow(window);
if (bind)
{
// Lifecycle Events
Expand All @@ -192,29 +197,39 @@ internal void HandleWindowEvents(Window window, bool bind = true)
window.ModalPopped += OnWindowOnModalPopped;
window.PopCanceled += OnWindowOnPopCanceled;
}
else
{
// Lifecycle events caused by user action
window.Activated -= OnWindowOnActivated;
window.Deactivated -= OnWindowOnDeactivated;
window.Stopped -= OnWindowOnStopped;
window.Resumed -= OnWindowOnResumed;

// System generated lifecycle events
window.Created -= OnWindowOnCreated;
window.Destroying -= OnWindowOnDestroying;
window.Backgrounding -= OnWindowOnBackgrounding;
window.DisplayDensityChanged -= OnWindowOnDisplayDensityChanged;
}

// Navigation events
window.ModalPushed -= OnWindowOnModalPushed;
window.ModalPopped -= OnWindowOnModalPopped;
window.PopCanceled -= OnWindowOnPopCanceled;
}
private void UnhookWindow(Window window)
{
// Lifecycle events caused by user action
window.Activated -= OnWindowOnActivated;
window.Deactivated -= OnWindowOnDeactivated;
window.Stopped -= OnWindowOnStopped;
window.Resumed -= OnWindowOnResumed;

// System generated lifecycle events
window.Created -= OnWindowOnCreated;
window.Destroying -= OnWindowOnDestroying;
window.Backgrounding -= OnWindowOnBackgrounding;
window.DisplayDensityChanged -= OnWindowOnDisplayDensityChanged;

// Navigation events
window.ModalPushed -= OnWindowOnModalPushed;
window.ModalPopped -= OnWindowOnModalPopped;
window.PopCanceled -= OnWindowOnPopCanceled;
}

internal void HandleElementEvents(Element element, bool bind = true)
{
// we always unbind the element first to ensure we don't have any sticky or repeat hooks
// Rendering events
element.ChildAdded -= OnElementOnChildAdded;
element.ChildRemoved -= OnElementOnChildRemoved;
element.ParentChanged -= OnElementOnParentChanged;

// BindableObject events
element.BindingContextChanged -= OnElementOnBindingContextChanged;

if (bind)
{
// Rendering events
Expand All @@ -229,34 +244,30 @@ internal void HandleElementEvents(Element element, bool bind = true)
// BindableObject events
element.BindingContextChanged += OnElementOnBindingContextChanged;
}
else
{
// Rendering events
element.ChildAdded -= OnElementOnChildAdded;
element.ChildRemoved -= OnElementOnChildRemoved;
element.ParentChanged -= OnElementOnParentChanged;

// BindableObject events
element.BindingContextChanged -= OnElementOnBindingContextChanged;
}
}

internal void HandleVisualElementEvents(VisualElement element, bool bind = true)
{
element.Focused -= OnElementOnFocused;
element.Unfocused -= OnElementOnUnfocused;

if (bind)
{
element.Focused += OnElementOnFocused;
element.Unfocused += OnElementOnUnfocused;
}
else
{
element.Focused -= OnElementOnFocused;
element.Unfocused -= OnElementOnUnfocused;
}
}

internal void HandleShellEvents(Shell shell, bool bind = true)
{
// Navigation events
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/navigation
shell.Navigating -= OnShellOnNavigating;
shell.Navigated -= OnShellOnNavigated;

// A Shell is also a Page
HandlePageEvents(shell, bind: false);

if (bind)
{
// Navigation events
Expand All @@ -267,20 +278,23 @@ internal void HandleShellEvents(Shell shell, bool bind = true)
// A Shell is also a Page
HandlePageEvents(shell);
}
else
{
// Navigation events
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/navigation
shell.Navigating -= OnShellOnNavigating;
shell.Navigated -= OnShellOnNavigated;

// A Shell is also a Page
HandlePageEvents(shell, bind: false);
}
}

internal void HandlePageEvents(Page page, bool bind = true)
{
// Lifecycle events
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/lifecycle
page.Appearing -= OnPageOnAppearing;
page.Disappearing -= OnPageOnDisappearing;

// Navigation events
// https://github.com/dotnet/docs-maui/issues/583
page.NavigatedTo -= OnPageOnNavigatedTo;

// Layout changed event
// https://docs.microsoft.com/dotnet/api/xamarin.forms.ilayout.layoutchanged
page.LayoutChanged -= OnPageOnLayoutChanged;

if (bind)
{
// Lifecycle events
Expand All @@ -296,21 +310,6 @@ internal void HandlePageEvents(Page page, bool bind = true)
// https://docs.microsoft.com/dotnet/api/xamarin.forms.ilayout.layoutchanged
page.LayoutChanged += OnPageOnLayoutChanged;
}
else
{
// Lifecycle events
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/lifecycle
page.Appearing -= OnPageOnAppearing;
page.Disappearing -= OnPageOnDisappearing;

// Navigation events
// https://github.com/dotnet/docs-maui/issues/583
page.NavigatedTo -= OnPageOnNavigatedTo;

// Layout changed event
// https://docs.microsoft.com/dotnet/api/xamarin.forms.ilayout.layoutchanged
page.LayoutChanged -= OnPageOnLayoutChanged;
}
}

// Application Events
Expand Down
Loading