Skip to content

Commit f92814d

Browse files
authored
Prevent leaking event hooks in our MauiEventHandler (#4159)
* Always unhook events first to ensure any recalls due to collectionview cell reuse/etc don't cause us to hook event multiple times * Update CHANGELOG.md
1 parent 8abc247 commit f92814d

File tree

2 files changed

+71
-71
lines changed

2 files changed

+71
-71
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Fixes
1212

1313
- Work around iOS SHA1 bug ([#4143](https://github.com/getsentry/sentry-dotnet/pull/4143))
14+
- Prevent Auto Breadcrumbs Event Binder from leaking and rebinding events ([#4159](https://github.com/getsentry/sentry-dotnet/pull/4159))
1415
- 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))
1516

1617
### Dependencies

src/Sentry.Maui/Internal/MauiEventsBinder.cs

Lines changed: 70 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ public MauiEventsBinder(IHub hub, IOptions<SentryMauiOptions> options, IEnumerab
3434

3535
public void HandleApplicationEvents(Application application, bool bind = true)
3636
{
37+
// we always unbind first to ensure no previous hooks
38+
UnbindApplication(application);
39+
3740
if (bind)
3841
{
3942
// Attach element events to all existing descendants (skip the application itself)
@@ -65,22 +68,23 @@ public void HandleApplicationEvents(Application application, bool bind = true)
6568
// https://docs.microsoft.com/dotnet/maui/user-interface/system-theme-changes#react-to-theme-changes
6669
application.RequestedThemeChanged += OnApplicationOnRequestedThemeChanged;
6770
}
68-
else
69-
{
70-
application.DescendantAdded -= OnApplicationOnDescendantAdded;
71-
application.DescendantRemoved -= OnApplicationOnDescendantRemoved;
71+
}
7272

73-
HandleElementEvents(application, bind: false);
73+
private void UnbindApplication(Application application)
74+
{
75+
application.DescendantAdded -= OnApplicationOnDescendantAdded;
76+
application.DescendantRemoved -= OnApplicationOnDescendantRemoved;
7477

75-
// Navigation events
76-
application.PageAppearing -= OnApplicationOnPageAppearing;
77-
application.PageDisappearing -= OnApplicationOnPageDisappearing;
78-
application.ModalPushed -= OnApplicationOnModalPushed;
79-
application.ModalPopped -= OnApplicationOnModalPopped;
78+
HandleElementEvents(application, bind: false);
8079

81-
// Theme changed event
82-
application.RequestedThemeChanged -= OnApplicationOnRequestedThemeChanged;
83-
}
80+
// Navigation events
81+
application.PageAppearing -= OnApplicationOnPageAppearing;
82+
application.PageDisappearing -= OnApplicationOnPageDisappearing;
83+
application.ModalPushed -= OnApplicationOnModalPushed;
84+
application.ModalPopped -= OnApplicationOnModalPopped;
85+
86+
// Theme changed event
87+
application.RequestedThemeChanged -= OnApplicationOnRequestedThemeChanged;
8488
}
8589

8690
internal void OnApplicationOnDescendantAdded(object? _, ElementEventArgs e)
@@ -169,6 +173,7 @@ internal void OnApplicationOnDescendantRemoved(object? _, ElementEventArgs e)
169173

170174
internal void HandleWindowEvents(Window window, bool bind = true)
171175
{
176+
UnhookWindow(window);
172177
if (bind)
173178
{
174179
// Lifecycle Events
@@ -192,29 +197,39 @@ internal void HandleWindowEvents(Window window, bool bind = true)
192197
window.ModalPopped += OnWindowOnModalPopped;
193198
window.PopCanceled += OnWindowOnPopCanceled;
194199
}
195-
else
196-
{
197-
// Lifecycle events caused by user action
198-
window.Activated -= OnWindowOnActivated;
199-
window.Deactivated -= OnWindowOnDeactivated;
200-
window.Stopped -= OnWindowOnStopped;
201-
window.Resumed -= OnWindowOnResumed;
202-
203-
// System generated lifecycle events
204-
window.Created -= OnWindowOnCreated;
205-
window.Destroying -= OnWindowOnDestroying;
206-
window.Backgrounding -= OnWindowOnBackgrounding;
207-
window.DisplayDensityChanged -= OnWindowOnDisplayDensityChanged;
200+
}
208201

209-
// Navigation events
210-
window.ModalPushed -= OnWindowOnModalPushed;
211-
window.ModalPopped -= OnWindowOnModalPopped;
212-
window.PopCanceled -= OnWindowOnPopCanceled;
213-
}
202+
private void UnhookWindow(Window window)
203+
{
204+
// Lifecycle events caused by user action
205+
window.Activated -= OnWindowOnActivated;
206+
window.Deactivated -= OnWindowOnDeactivated;
207+
window.Stopped -= OnWindowOnStopped;
208+
window.Resumed -= OnWindowOnResumed;
209+
210+
// System generated lifecycle events
211+
window.Created -= OnWindowOnCreated;
212+
window.Destroying -= OnWindowOnDestroying;
213+
window.Backgrounding -= OnWindowOnBackgrounding;
214+
window.DisplayDensityChanged -= OnWindowOnDisplayDensityChanged;
215+
216+
// Navigation events
217+
window.ModalPushed -= OnWindowOnModalPushed;
218+
window.ModalPopped -= OnWindowOnModalPopped;
219+
window.PopCanceled -= OnWindowOnPopCanceled;
214220
}
215221

216222
internal void HandleElementEvents(Element element, bool bind = true)
217223
{
224+
// we always unbind the element first to ensure we don't have any sticky or repeat hooks
225+
// Rendering events
226+
element.ChildAdded -= OnElementOnChildAdded;
227+
element.ChildRemoved -= OnElementOnChildRemoved;
228+
element.ParentChanged -= OnElementOnParentChanged;
229+
230+
// BindableObject events
231+
element.BindingContextChanged -= OnElementOnBindingContextChanged;
232+
218233
if (bind)
219234
{
220235
// Rendering events
@@ -229,34 +244,30 @@ internal void HandleElementEvents(Element element, bool bind = true)
229244
// BindableObject events
230245
element.BindingContextChanged += OnElementOnBindingContextChanged;
231246
}
232-
else
233-
{
234-
// Rendering events
235-
element.ChildAdded -= OnElementOnChildAdded;
236-
element.ChildRemoved -= OnElementOnChildRemoved;
237-
element.ParentChanged -= OnElementOnParentChanged;
238-
239-
// BindableObject events
240-
element.BindingContextChanged -= OnElementOnBindingContextChanged;
241-
}
242247
}
243248

244249
internal void HandleVisualElementEvents(VisualElement element, bool bind = true)
245250
{
251+
element.Focused -= OnElementOnFocused;
252+
element.Unfocused -= OnElementOnUnfocused;
253+
246254
if (bind)
247255
{
248256
element.Focused += OnElementOnFocused;
249257
element.Unfocused += OnElementOnUnfocused;
250258
}
251-
else
252-
{
253-
element.Focused -= OnElementOnFocused;
254-
element.Unfocused -= OnElementOnUnfocused;
255-
}
256259
}
257260

258261
internal void HandleShellEvents(Shell shell, bool bind = true)
259262
{
263+
// Navigation events
264+
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/navigation
265+
shell.Navigating -= OnShellOnNavigating;
266+
shell.Navigated -= OnShellOnNavigated;
267+
268+
// A Shell is also a Page
269+
HandlePageEvents(shell, bind: false);
270+
260271
if (bind)
261272
{
262273
// Navigation events
@@ -267,20 +278,23 @@ internal void HandleShellEvents(Shell shell, bool bind = true)
267278
// A Shell is also a Page
268279
HandlePageEvents(shell);
269280
}
270-
else
271-
{
272-
// Navigation events
273-
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/navigation
274-
shell.Navigating -= OnShellOnNavigating;
275-
shell.Navigated -= OnShellOnNavigated;
276-
277-
// A Shell is also a Page
278-
HandlePageEvents(shell, bind: false);
279-
}
280281
}
281282

282283
internal void HandlePageEvents(Page page, bool bind = true)
283284
{
285+
// Lifecycle events
286+
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/lifecycle
287+
page.Appearing -= OnPageOnAppearing;
288+
page.Disappearing -= OnPageOnDisappearing;
289+
290+
// Navigation events
291+
// https://github.com/dotnet/docs-maui/issues/583
292+
page.NavigatedTo -= OnPageOnNavigatedTo;
293+
294+
// Layout changed event
295+
// https://docs.microsoft.com/dotnet/api/xamarin.forms.ilayout.layoutchanged
296+
page.LayoutChanged -= OnPageOnLayoutChanged;
297+
284298
if (bind)
285299
{
286300
// Lifecycle events
@@ -296,21 +310,6 @@ internal void HandlePageEvents(Page page, bool bind = true)
296310
// https://docs.microsoft.com/dotnet/api/xamarin.forms.ilayout.layoutchanged
297311
page.LayoutChanged += OnPageOnLayoutChanged;
298312
}
299-
else
300-
{
301-
// Lifecycle events
302-
// https://docs.microsoft.com/dotnet/maui/fundamentals/shell/lifecycle
303-
page.Appearing -= OnPageOnAppearing;
304-
page.Disappearing -= OnPageOnDisappearing;
305-
306-
// Navigation events
307-
// https://github.com/dotnet/docs-maui/issues/583
308-
page.NavigatedTo -= OnPageOnNavigatedTo;
309-
310-
// Layout changed event
311-
// https://docs.microsoft.com/dotnet/api/xamarin.forms.ilayout.layoutchanged
312-
page.LayoutChanged -= OnPageOnLayoutChanged;
313-
}
314313
}
315314

316315
// Application Events

0 commit comments

Comments
 (0)