Skip to content

Commit 3d91b35

Browse files
jonathanpeppersjonathanpeppers
committed
[ios] fix memory leak in SearchBar
Context: #16346 This addresses the memory leak discovered by: src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak. src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak. src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak. src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method. I could reproduce a leak in a test such as: await InvokeOnMainThreadAsync(() => { var layout = new Grid(); var searchBar = new SearchBar(); layout.Add(searchBar); var handler = CreateHandler<LayoutHandler>(layout); viewReference = new WeakReference(searchBar); handlerReference = new WeakReference(searchBar.Handler); platformViewReference = new WeakReference(searchBar.Handler.PlatformView); }); await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference); Assert.False(viewReference.IsAlive, "SearchBar should not be alive!"); Assert.False(handlerReference.IsAlive, "Handler should not be alive!"); Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!"); Solved the issues by making a non-NSObject `MauiSearchBarProxy` class. I found & fixed a couple related issues: * `SearchBarExtensions.GetSearchTextField()` would have always thrown `StackOverlowException` if iOS was less than 13? It just called itself?!? * Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the event from the `UITextField` directly instead.
1 parent 22fab2d commit 3d91b35

File tree

4 files changed

+115
-63
lines changed

4 files changed

+115
-63
lines changed

‎src/Controls/tests/DeviceTests/Memory/MemoryTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ void SetupBuilder()
2727
handlers.AddHandler<Image, ImageHandler>();
2828
handlers.AddHandler<RefreshView, RefreshViewHandler>();
2929
handlers.AddHandler<IScrollView, ScrollViewHandler>();
30+
handlers.AddHandler<SearchBar, SearchBarHandler>();
3031
handlers.AddHandler<SwipeView, SwipeViewHandler>();
3132
handlers.AddHandler<TimePicker, TimePickerHandler>();
3233
});
@@ -46,6 +47,7 @@ void SetupBuilder()
4647
[InlineData(typeof(Picker))]
4748
[InlineData(typeof(RefreshView))]
4849
[InlineData(typeof(ScrollView))]
50+
[InlineData(typeof(SearchBar))]
4951
[InlineData(typeof(SwipeView))]
5052
[InlineData(typeof(TimePicker))]
5153
public async Task HandlerDoesNotLeak(Type type)

‎src/Core/src/Handlers/SearchBar/SearchBarHandler.iOS.cs

Lines changed: 91 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ namespace Microsoft.Maui.Handlers
88
public partial class SearchBarHandler : ViewHandler<ISearchBar, MauiSearchBar>
99
{
1010
UITextField? _editor;
11+
readonly MauiSearchBarProxy _proxy = new();
1112

1213
public UITextField? QueryEditor => _editor;
1314

@@ -23,35 +24,14 @@ protected override MauiSearchBar CreatePlatformView()
2324

2425
protected override void ConnectHandler(MauiSearchBar platformView)
2526
{
26-
platformView.CancelButtonClicked += OnCancelClicked;
27-
platformView.SearchButtonClicked += OnSearchButtonClicked;
28-
platformView.TextSetOrChanged += OnTextPropertySet;
29-
platformView.OnMovedToWindow += OnMovedToWindow;
30-
platformView.ShouldChangeTextInRange += ShouldChangeText;
31-
platformView.OnEditingStarted += OnEditingStarted;
32-
platformView.EditingChanged += OnEditingChanged;
33-
platformView.OnEditingStopped += OnEditingStopped;
27+
_proxy.Connect(this, VirtualView, platformView);
3428

3529
base.ConnectHandler(platformView);
3630
}
3731

38-
void OnMovedToWindow(object? sender, EventArgs e)
39-
{
40-
// The cancel button doesn't exist until the control has moved to the window
41-
// so we fire this off again so it can set the color
42-
UpdateValue(nameof(ISearchBar.CancelButtonColor));
43-
}
44-
4532
protected override void DisconnectHandler(MauiSearchBar platformView)
4633
{
47-
platformView.CancelButtonClicked -= OnCancelClicked;
48-
platformView.SearchButtonClicked -= OnSearchButtonClicked;
49-
platformView.TextSetOrChanged -= OnTextPropertySet;
50-
platformView.ShouldChangeTextInRange -= ShouldChangeText;
51-
platformView.OnMovedToWindow -= OnMovedToWindow;
52-
platformView.OnEditingStarted -= OnEditingStarted;
53-
platformView.EditingChanged -= OnEditingChanged;
54-
platformView.OnEditingStopped -= OnEditingStopped;
34+
_proxy.Disconnect(platformView);
5535

5636
base.DisconnectHandler(platformView);
5737
}
@@ -166,56 +146,107 @@ public static void MapKeyboard(ISearchBarHandler handler, ISearchBar searchBar)
166146
handler.PlatformView?.UpdateKeyboard(searchBar);
167147
}
168148

169-
void OnCancelClicked(object? sender, EventArgs args)
149+
void UpdateCancelButtonVisibility()
170150
{
171-
if (VirtualView != null)
172-
VirtualView.Text = string.Empty;
151+
if (PlatformView.ShowsCancelButton != VirtualView.ShouldShowCancelButton())
152+
UpdateValue(nameof(ISearchBar.CancelButtonColor));
173153
}
174154

175-
void OnSearchButtonClicked(object? sender, EventArgs e)
155+
class MauiSearchBarProxy
176156
{
177-
VirtualView?.SearchButtonPressed();
178-
}
157+
WeakReference<SearchBarHandler>? _handler;
158+
WeakReference<ISearchBar>? _virtualView;
179159

180-
void OnTextPropertySet(object? sender, UISearchBarTextChangedEventArgs a)
181-
{
182-
VirtualView.UpdateText(a.SearchText);
160+
ISearchBar? VirtualView => _virtualView is not null && _virtualView.TryGetTarget(out var v) ? v : null;
183161

184-
UpdateCancelButtonVisibility();
185-
}
162+
public void Connect(SearchBarHandler handler, ISearchBar virtualView, MauiSearchBar platformView)
163+
{
164+
_handler = new(handler);
165+
_virtualView = new(virtualView);
166+
167+
platformView.CancelButtonClicked += OnCancelClicked;
168+
platformView.SearchButtonClicked += OnSearchButtonClicked;
169+
platformView.TextSetOrChanged += OnTextPropertySet;
170+
platformView.OnMovedToWindow += OnMovedToWindow;
171+
platformView.ShouldChangeTextInRange += ShouldChangeText;
172+
platformView.OnEditingStarted += OnEditingStarted;
173+
platformView.EditingChanged += OnEditingChanged;
174+
platformView.OnEditingStopped += OnEditingStopped;
175+
}
186176

187-
bool ShouldChangeText(UISearchBar searchBar, NSRange range, string text)
188-
{
189-
var newLength = searchBar?.Text?.Length + text.Length - range.Length;
190-
return newLength <= VirtualView?.MaxLength;
191-
}
177+
public void Disconnect(MauiSearchBar platformView)
178+
{
179+
_virtualView = null;
180+
181+
platformView.CancelButtonClicked -= OnCancelClicked;
182+
platformView.SearchButtonClicked -= OnSearchButtonClicked;
183+
platformView.TextSetOrChanged -= OnTextPropertySet;
184+
platformView.ShouldChangeTextInRange -= ShouldChangeText;
185+
platformView.OnMovedToWindow -= OnMovedToWindow;
186+
platformView.OnEditingStarted -= OnEditingStarted;
187+
platformView.EditingChanged -= OnEditingChanged;
188+
platformView.OnEditingStopped -= OnEditingStopped;
189+
}
192190

193-
void OnEditingStarted(object? sender, EventArgs e)
194-
{
195-
if (VirtualView is not null)
196-
VirtualView.IsFocused = true;
197-
}
191+
void OnMovedToWindow(object? sender, EventArgs e)
192+
{
193+
// The cancel button doesn't exist until the control has moved to the window
194+
// so we fire this off again so it can set the color
195+
if (_handler is not null && _handler.TryGetTarget(out var handler))
196+
{
197+
handler.UpdateValue(nameof(ISearchBar.CancelButtonColor));
198+
}
199+
}
198200

199-
void OnEditingChanged(object? sender, EventArgs e)
200-
{
201-
if (VirtualView == null || _editor == null)
202-
return;
201+
void OnCancelClicked(object? sender, EventArgs args)
202+
{
203+
if (VirtualView is ISearchBar virtualView)
204+
virtualView.Text = string.Empty;
205+
}
203206

204-
VirtualView.UpdateText(_editor.Text);
207+
void OnSearchButtonClicked(object? sender, EventArgs e)
208+
{
209+
VirtualView?.SearchButtonPressed();
210+
}
205211

206-
UpdateCancelButtonVisibility();
207-
}
212+
void OnTextPropertySet(object? sender, UISearchBarTextChangedEventArgs a)
213+
{
214+
if (VirtualView is ISearchBar virtualView)
215+
{
216+
virtualView.UpdateText(a.SearchText);
217+
218+
if (_handler is not null && _handler.TryGetTarget(out var handler))
219+
{
220+
handler.UpdateCancelButtonVisibility();
221+
}
222+
}
223+
}
208224

209-
void OnEditingStopped(object? sender, EventArgs e)
210-
{
211-
if (VirtualView is not null)
212-
VirtualView.IsFocused = false;
213-
}
225+
bool ShouldChangeText(UISearchBar searchBar, NSRange range, string text)
226+
{
227+
var newLength = searchBar?.Text?.Length + text.Length - range.Length;
228+
return newLength <= VirtualView?.MaxLength;
229+
}
214230

215-
void UpdateCancelButtonVisibility()
216-
{
217-
if (PlatformView.ShowsCancelButton != VirtualView.ShouldShowCancelButton())
218-
UpdateValue(nameof(ISearchBar.CancelButtonColor));
231+
void OnEditingStarted(object? sender, EventArgs e)
232+
{
233+
if (VirtualView is ISearchBar virtualView)
234+
virtualView.IsFocused = true;
235+
}
236+
237+
void OnEditingChanged(object? sender, EventArgs e)
238+
{
239+
if (_handler is not null && _handler.TryGetTarget(out var handler))
240+
{
241+
handler.UpdateCancelButtonVisibility();
242+
}
243+
}
244+
245+
void OnEditingStopped(object? sender, EventArgs e)
246+
{
247+
if (VirtualView is ISearchBar virtualView)
248+
virtualView.IsFocused = false;
249+
}
219250
}
220251
}
221252
}

‎src/Core/src/Platform/iOS/MauiSearchBar.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
using System;
2-
using System.ComponentModel.Design;
2+
using System.Diagnostics.CodeAnalysis;
33
using System.Drawing;
44
using CoreGraphics;
55
using Foundation;
@@ -33,6 +33,7 @@ protected internal MauiSearchBar(NativeHandle handle) : base(handle)
3333
// Native Changed doesn't fire when the Text Property is set in code
3434
// We use this event as a way to fire changes whenever the Text changes
3535
// via code or user interaction.
36+
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
3637
public event EventHandler<UISearchBarTextChangedEventArgs>? TextSetOrChanged;
3738

3839
public override string? Text
@@ -51,7 +52,9 @@ public override string? Text
5152
}
5253
}
5354

55+
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
5456
internal event EventHandler? OnMovedToWindow;
57+
[UnconditionalSuppressMessage("Memory", "MA0001", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
5558
internal event EventHandler? EditingChanged;
5659

5760
public override void WillMoveToWindow(UIWindow? window)

‎src/Core/src/Platform/iOS/SearchBarExtensions.cs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,25 @@ public static class SearchBarExtensions
1010
internal static UITextField? GetSearchTextField(this UISearchBar searchBar)
1111
{
1212
if (OperatingSystem.IsIOSVersionAtLeast(13))
13+
{
1314
return searchBar.SearchTextField;
14-
else
15-
return searchBar.GetSearchTextField();
15+
}
16+
17+
// Search Subviews up to two levels deep
18+
// https://stackoverflow.com/a/58056700
19+
foreach (var child in searchBar.Subviews)
20+
{
21+
if (child is UITextField childTextField)
22+
return childTextField;
23+
24+
foreach (var grandChild in child.Subviews)
25+
{
26+
if (grandChild is UITextField grandChildTextField)
27+
return grandChildTextField;
28+
}
29+
}
30+
31+
return null;
1632
}
1733

1834
internal static void UpdateBackground(this UISearchBar uiSearchBar, ISearchBar searchBar)

0 commit comments

Comments
 (0)