Skip to content

[ios] fix memory leaks in GestureRecognizers #21089

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

Closed
wants to merge 1 commit into from
Closed

[ios] fix memory leaks in GestureRecognizers #21089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 37 additions & 28 deletions src/Controls/src/Core/Platform/GestureManager/GesturePlatformManager.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.Versioning;
using CoreGraphics;
using Foundation;
Expand All @@ -20,9 +21,9 @@ class GesturePlatformManager : IDisposable
{
readonly NotifyCollectionChangedEventHandler _collectionChangedHandler;

readonly Dictionary<IGestureRecognizer, List<UIGestureRecognizer?>> _gestureRecognizers = new Dictionary<IGestureRecognizer, List<UIGestureRecognizer?>>();
readonly ConditionalWeakTable<IGestureRecognizer, List<UIGestureRecognizer?>> _gestureRecognizers = new();
readonly List<INativeObject> _interactions = new List<INativeObject>();
readonly IPlatformViewHandler _handler;
readonly WeakReference<IPlatformViewHandler> _handler;

bool _disposed;
WeakReference<PlatformView>? _platformView;
Expand All @@ -38,17 +39,18 @@ public GesturePlatformManager(IViewHandler handler)
if (handler == null)
throw new ArgumentNullException(nameof(handler));

_handler = (IPlatformViewHandler)handler;
var platformHandler = (IPlatformViewHandler)handler;

if (_handler?.ToPlatform() is not PlatformView target)
if (platformHandler?.ToPlatform() is not PlatformView target)
throw new ArgumentNullException(nameof(handler.PlatformView));

_handler = new WeakReference<IPlatformViewHandler>(platformHandler);
_platformView = new WeakReference<PlatformView>(target);

_collectionChangedHandler = GestureRecognizersOnCollectionChanged;

// In XF this was called inside ViewDidLoad
if (_handler.VirtualView is View view)
if (platformHandler.VirtualView is View view)
OnElementChanged(this, new VisualElementChangedEventArgs(null, view));
else
throw new ArgumentNullException(nameof(handler.VirtualView));
Expand All @@ -64,8 +66,14 @@ protected virtual PlatformView? PlatformView
}
}

IPlatformViewHandler? Handler => _handler.TryGetTarget(out var handler) ? handler : null;

IView? VirtualView => Handler?.VirtualView;

UIWindow? PlatformWindow => Handler?.MauiContext?.GetPlatformWindow();

ObservableCollection<IGestureRecognizer>? ElementGestureRecognizers =>
(_handler.VirtualView as Element)?.GetCompositeGestureRecognizers() as ObservableCollection<IGestureRecognizer>;
(VirtualView as Element)?.GetCompositeGestureRecognizers() as ObservableCollection<IGestureRecognizer>;

internal void Disconnect()
{
Expand Down Expand Up @@ -142,7 +150,7 @@ static void ProcessRecognizerHandlerTap(
{
var recognizer = weakRecognizer.Target as IGestureRecognizer;
var eventTracker = weakEventTracker.Target as GesturePlatformManager;
var view = eventTracker?._handler?.VirtualView as View;
var view = eventTracker?.VirtualView as View;

WeakReference? weakPlatformRecognizer = null;
if (uITapGestureRecognizer != null)
Expand Down Expand Up @@ -178,7 +186,7 @@ static void ProcessRecognizerHandlerTap(
static Point? CalculatePosition(IElement? element, CGPoint originPoint, WeakReference? weakPlatformRecognizer, WeakReference weakEventTracker)
{
var eventTracker = weakEventTracker.Target as GesturePlatformManager;
var virtualView = eventTracker?._handler?.VirtualView as View;
var virtualView = eventTracker?.VirtualView as View;
var platformRecognizer = weakPlatformRecognizer?.Target as UIGestureRecognizer;
var platformView = element?.ToPlatform();

Expand Down Expand Up @@ -257,7 +265,7 @@ static void ProcessRecognizerHandlerTap(
{
var swipeGestureRecognizer = weakRecognizer.Target as SwipeGestureRecognizer;
var eventTracker = weakEventTracker.Target as GesturePlatformManager;
var view = eventTracker?._handler.VirtualView as View;
var view = eventTracker?.VirtualView as View;

if (swipeGestureRecognizer != null && view != null)
swipeGestureRecognizer.SendSwiped(view, direction);
Expand All @@ -274,7 +282,7 @@ static void ProcessRecognizerHandlerTap(
{
if (weakRecognizer.Target is IPinchGestureController pinchGestureRecognizer &&
weakEventTracker.Target is GesturePlatformManager eventTracker &&
eventTracker._handler?.VirtualView is View view &&
eventTracker?.VirtualView is View view &&
UIApplication.SharedApplication.GetKeyWindow() is UIWindow window)
{
var oldScale = eventTracker._previousScale;
Expand Down Expand Up @@ -332,20 +340,20 @@ weakEventTracker.Target is GesturePlatformManager eventTracker &&
var uiRecognizer = CreatePanRecognizer(panRecognizer.TouchPoints, r =>
{
var eventTracker = weakEventTracker.Target as GesturePlatformManager;
var view = eventTracker?._handler?.VirtualView as View;
var view = eventTracker?.VirtualView as View;

var panGestureRecognizer = weakRecognizer.Target as IPanGestureController;
if (panGestureRecognizer != null && view != null)
{
switch (r.State)
{
case UIGestureRecognizerState.Began:
if (r.NumberOfTouches != panRecognizer.TouchPoints)
if (r.NumberOfTouches != ((PanGestureRecognizer)panGestureRecognizer).TouchPoints)
return;
panGestureRecognizer.SendPanStarted(view, PanGestureRecognizer.CurrentId.Value);
break;
case UIGestureRecognizerState.Changed:
if (r.NumberOfTouches != panRecognizer.TouchPoints)
if (r.NumberOfTouches != ((PanGestureRecognizer)panGestureRecognizer).TouchPoints)
{
r.State = UIGestureRecognizerState.Ended;
panGestureRecognizer.SendPanCompleted(view, PanGestureRecognizer.CurrentId.Value);
Expand All @@ -361,7 +369,7 @@ weakEventTracker.Target is GesturePlatformManager eventTracker &&
PanGestureRecognizer.CurrentId.Increment();
break;
case UIGestureRecognizerState.Ended:
if (r.NumberOfTouches != panRecognizer.TouchPoints)
if (r.NumberOfTouches != ((PanGestureRecognizer)panGestureRecognizer).TouchPoints)
{
panGestureRecognizer.SendPanCompleted(view, PanGestureRecognizer.CurrentId.Value);
PanGestureRecognizer.CurrentId.Increment();
Expand Down Expand Up @@ -412,8 +420,8 @@ UISwipeGestureRecognizer CreateSwipeRecognizer(SwipeDirection direction, Action<
{
if (weakRecognizer.Target is PointerGestureRecognizer pointerGestureRecognizer &&
weakEventTracker.Target is GesturePlatformManager eventTracker &&
eventTracker._handler?.VirtualView is View view &&
eventTracker._handler?.MauiContext?.GetPlatformWindow() is UIWindow window)
eventTracker?.VirtualView is View view &&
eventTracker?.PlatformWindow is UIWindow window)
{
var originPoint = pointerGesture.LocationInView(eventTracker?.PlatformView);
var platformPointerArgs = new PlatformPointerEventArgs(pointerGesture.View, pointerGesture);
Expand Down Expand Up @@ -592,7 +600,7 @@ void LoadRecognizers()
bool dropFound = false;

if (PlatformView != null &&
_handler.VirtualView is View v &&
VirtualView is View v &&
v.TapGestureRecognizerNeedsDelegate() &&
(PlatformView.AccessibilityTraits & UIAccessibilityTrait.Button) != UIAccessibilityTrait.Button)
{
Expand All @@ -615,7 +623,7 @@ _handler.VirtualView is View v &&
{
IGestureRecognizer recognizer = ElementGestureRecognizers[i];

if (_gestureRecognizers.ContainsKey(recognizer))
if (_gestureRecognizers.TryGetValue(recognizer, out _))
continue;

if (TryGetTapGestureRecognizer(recognizer, out TapGestureRecognizer? tapGestureRecognizer) &&
Expand All @@ -637,7 +645,7 @@ _handler.VirtualView is View v &&
if (OperatingSystem.IsIOSVersionAtLeast(11) && recognizer is DragGestureRecognizer)
{
dragFound = true;
_dragAndDropDelegate = _dragAndDropDelegate ?? new DragAndDropDelegate(_handler);
_dragAndDropDelegate = _dragAndDropDelegate ?? new DragAndDropDelegate(Handler);
if (uIDragInteraction == null && PlatformView != null)
{
var interaction = new UIDragInteraction(_dragAndDropDelegate);
Expand All @@ -650,7 +658,7 @@ _handler.VirtualView is View v &&
if (OperatingSystem.IsIOSVersionAtLeast(11) && recognizer is DropGestureRecognizer)
{
dropFound = true;
_dragAndDropDelegate = _dragAndDropDelegate ?? new DragAndDropDelegate(_handler);
_dragAndDropDelegate = _dragAndDropDelegate ?? new DragAndDropDelegate(Handler);
if (uIDropInteraction == null && PlatformView != null)
{
var interaction = new UIDropInteraction(_dragAndDropDelegate);
Expand All @@ -664,7 +672,7 @@ _handler.VirtualView is View v &&
if (nativeRecognizers is null)
continue;

_gestureRecognizers[recognizer] = nativeRecognizers;
_gestureRecognizers.AddOrUpdate(recognizer, nativeRecognizers);
foreach (UIGestureRecognizer? nativeRecognizer in nativeRecognizers)
{
if (nativeRecognizer != null && PlatformView != null)
Expand All @@ -685,18 +693,19 @@ _handler.VirtualView is View v &&
PlatformView.RemoveInteraction(uIDropInteraction);
}

var toRemove = new List<IGestureRecognizer>();
var toRemove = new List<KeyValuePair<IGestureRecognizer, List<UIGestureRecognizer?>>>();

foreach (var key in _gestureRecognizers.Keys)
foreach (var pair in _gestureRecognizers)
{
if (!ElementGestureRecognizers.Contains(key))
toRemove.Add(key);
if (!ElementGestureRecognizers.Contains(pair.Key))
toRemove.Add(pair);
}

for (int i = 0; i < toRemove.Count; i++)
{
IGestureRecognizer gestureRecognizer = toRemove[i];
var uiRecognizers = _gestureRecognizers[gestureRecognizer];
var pair = toRemove[i];
IGestureRecognizer gestureRecognizer = pair.Key;
var uiRecognizers = pair.Value;
_gestureRecognizers.Remove(gestureRecognizer);

foreach (var uiRecognizer in uiRecognizers)
Expand Down Expand Up @@ -741,7 +750,7 @@ void OnTapGestureRecognizerPropertyChanged(object? sender, System.ComponentModel
bool ShouldReceiveTouch(UIGestureRecognizer recognizer, UITouch touch)
{
var platformView = PlatformView;
var virtualView = _handler?.VirtualView;
var virtualView = VirtualView;

if (virtualView == null || platformView == null)
{
Expand Down
30 changes: 30 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,36 @@ await InvokeOnMainThreadAsync(async () =>
await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
}

[Theory("Gesture Does Not Leak")]
[InlineData(typeof(DragGestureRecognizer))]
[InlineData(typeof(DropGestureRecognizer))]
[InlineData(typeof(PanGestureRecognizer))]
[InlineData(typeof(PinchGestureRecognizer))]
[InlineData(typeof(PointerGestureRecognizer))]
[InlineData(typeof(SwipeGestureRecognizer))]
[InlineData(typeof(TapGestureRecognizer))]
public async Task GestureDoesNotLeak(Type type)
{
SetupBuilder();

WeakReference viewReference = null;
WeakReference platformViewReference = null;
WeakReference handlerReference = null;

await InvokeOnMainThreadAsync(() =>
{
var view = new Label(); // Just using simplest view
view.GestureRecognizers.Add((GestureRecognizer)Activator.CreateInstance(type));

var handler = CreateHandler<LabelHandler>(view);
viewReference = new WeakReference(view);
handlerReference = new WeakReference(view.Handler);
platformViewReference = new WeakReference(view.Handler.PlatformView);
});

await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
}

#if IOS
[Fact]
public async Task ResignFirstResponderTouchGestureRecognizer()
Expand Down