From 40ba4cebf6656a2d54d6dfd1d03cf7590f31bfff Mon Sep 17 00:00:00 2001 From: Timothy Sturm Date: Thu, 26 Mar 2026 12:20:19 -0500 Subject: [PATCH 1/7] [iOS] Fix ObjectDisposedException in ContentView.RemoveContentMask Check Handle != IntPtr.Zero before calling RemoveFromSuperLayer() on _contentMask to prevent ObjectDisposedException when iOS has already deallocated the underlying CAShapeLayer during view hierarchy teardown. This follows the established disposal guard pattern used throughout the codebase (ViewRenderer.cs, LayoutFactory2.cs, CellRenderer.cs, etc.). Fixes a crash reported in Active Trader Pro on iOS/MacCatalyst where WillRemoveSubview triggers RemoveContentMask after the native layer has been collected. Related: #30861, #33818 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Core/src/Platform/iOS/ContentView.cs | 7 ++- .../ContentView/ContentViewTests.iOS.cs | 63 ++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/Core/src/Platform/iOS/ContentView.cs b/src/Core/src/Platform/iOS/ContentView.cs index 5f6f14398f95..f6069fa96d38 100644 --- a/src/Core/src/Platform/iOS/ContentView.cs +++ b/src/Core/src/Platform/iOS/ContentView.cs @@ -63,7 +63,10 @@ internal IBorderStroke? Clip void RemoveContentMask() { - _contentMask?.RemoveFromSuperLayer(); + if (_contentMask is not null && _contentMask.Handle != IntPtr.Zero) + { + _contentMask.RemoveFromSuperLayer(); + } _contentMask = null; } @@ -139,4 +142,4 @@ public override void WillRemoveSubview(UIView uiview) base.WillRemoveSubview(uiview); } } -} \ No newline at end of file +} diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index 956cc11f764b..2522a88ac57b 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -1,5 +1,10 @@ -using System.Threading.Tasks; +using System; +using System.Threading.Tasks; +using CoreAnimation; +using CoreGraphics; using Microsoft.Maui.DeviceTests.Stubs; +using Microsoft.Maui.Graphics; +using Microsoft.Maui.Platform; using UIKit; using Xunit; @@ -105,5 +110,61 @@ public async Task DoesNotPropagateToContentWithExplicitFlowDirection() Assert.Equal(UIUserInterfaceLayoutDirection.LeftToRight, labelFlowDirection); } + + [Fact] + public async Task RemoveContentMaskDoesNotThrowWhenDisposed() + { + // Verify that removing a subview with an active clip mask does not throw + // ObjectDisposedException when the underlying CAShapeLayer is already disposed. + // Regression test for https://github.com/FFIDX-Success/ATP-Support/issues/561 + await InvokeOnMainThreadAsync(() => + { + var contentView = new Microsoft.Maui.Platform.ContentView(); + contentView.Frame = new CGRect(0, 0, 200, 200); + + var content = new UIView { Tag = Microsoft.Maui.Platform.ContentView.ContentTag }; + content.Frame = new CGRect(0, 0, 200, 200); + contentView.AddSubview(content); + + // Set a clip to trigger _contentMask creation via UpdateClip + contentView.Clip = new BorderStrokeStub(); + contentView.LayoutSubviews(); + + // Dispose the content mask's native handle (simulating iOS deallocating the layer) + if (content.Layer.Mask is CAShapeLayer mask) + { + mask.Dispose(); + } + + // This should not throw ObjectDisposedException + var ex = Record.Exception(() => content.RemoveFromSuperview()); + Assert.Null(ex); + }); + } + + /// + /// Minimal IBorderStroke stub for testing clip mask creation. + /// + class BorderStrokeStub : IBorderStroke + { + public IShape Shape { get; set; } = new RectangleShape(); + public Paint Stroke { get; set; } + public double StrokeThickness { get; set; } = 1; + public LineCap StrokeLineCap { get; set; } + public LineJoin StrokeLineJoin { get; set; } + public float[] StrokeDashPattern { get; set; } + public float StrokeDashOffset { get; set; } + public float StrokeMiterLimit { get; set; } + } + + class RectangleShape : IShape + { + public PathF PathForBounds(Rect bounds) + { + var path = new PathF(); + path.AppendRectangle(bounds); + return path; + } + } } } From f81081f12500406ff41c3e08c6931237b74863bf Mon Sep 17 00:00:00 2001 From: Timothy Sturm Date: Fri, 27 Mar 2026 07:21:10 -0500 Subject: [PATCH 2/7] Address review feedback: make test assertions explicit - Replace conditional mask disposal with Assert.IsType to fail fast if the mask is not created (avoids false-positive) - Assert Handle == IntPtr.Zero after disposal to verify test setup - Remove customer issue link from test comment - Add reference to upstream dotnet/macios#10562 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Handlers/ContentView/ContentViewTests.iOS.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index 2522a88ac57b..ec1c65e49510 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -116,7 +116,7 @@ public async Task RemoveContentMaskDoesNotThrowWhenDisposed() { // Verify that removing a subview with an active clip mask does not throw // ObjectDisposedException when the underlying CAShapeLayer is already disposed. - // Regression test for https://github.com/FFIDX-Success/ATP-Support/issues/561 + // Related: https://github.com/dotnet/macios/issues/10562 await InvokeOnMainThreadAsync(() => { var contentView = new Microsoft.Maui.Platform.ContentView(); @@ -130,11 +130,11 @@ await InvokeOnMainThreadAsync(() => contentView.Clip = new BorderStrokeStub(); contentView.LayoutSubviews(); - // Dispose the content mask's native handle (simulating iOS deallocating the layer) - if (content.Layer.Mask is CAShapeLayer mask) - { - mask.Dispose(); - } + // Verify the mask was created, then dispose its native handle + // (simulating iOS deallocating the layer during view teardown) + var mask = Assert.IsType(content.Layer.Mask); + mask.Dispose(); + Assert.True(mask.Handle == IntPtr.Zero); // This should not throw ObjectDisposedException var ex = Record.Exception(() => content.RemoveFromSuperview()); From 0dc556eac25d5277f0e6c6053eee39149f0914bb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 29 Mar 2026 18:50:29 -0500 Subject: [PATCH 3/7] Fix device test: use Assert.IsAssignableFrom for CAShapeLayer subclass Assert.IsType requires an exact type match, but _contentMask is typed as StaticCAShapeLayer (a subclass of CAShapeLayer). This caused the RemoveContentMaskDoesNotThrowWhenDisposed test to fail on iOS and MacCatalyst with: 'Assert.IsType() Failure: Value is not the exact type' Also improve Assert.True to Assert.Equal for better diagnostic output on failure. Fixes: net10.0 iOS/MacCatalyst Helix Tests (Mono) device test failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Handlers/ContentView/ContentViewTests.iOS.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index ec1c65e49510..34b628846519 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -131,10 +131,13 @@ await InvokeOnMainThreadAsync(() => contentView.LayoutSubviews(); // Verify the mask was created, then dispose its native handle - // (simulating iOS deallocating the layer during view teardown) - var mask = Assert.IsType(content.Layer.Mask); + // (simulating iOS deallocating the layer during view teardown). + // _contentMask is a StaticCAShapeLayer (subclass of CAShapeLayer), so use + // IsAssignableFrom rather than IsType to accept subclasses. + Assert.IsAssignableFrom(content.Layer.Mask); + var mask = (CAShapeLayer)content.Layer.Mask!; mask.Dispose(); - Assert.True(mask.Handle == IntPtr.Zero); + Assert.Equal(IntPtr.Zero, mask.Handle); // This should not throw ObjectDisposedException var ex = Record.Exception(() => content.RemoveFromSuperview()); From ca92ceffeb203562a8e233c5b771b6f22c2824de Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 30 Mar 2026 08:40:58 -0500 Subject: [PATCH 4/7] Fix CS1503 build error: use Assert.True for nint/IntPtr Handle comparison Assert.Equal(IntPtr.Zero, mask.Handle) fails on Apple TFMs because mask.Handle returns nint (NativeHandle) and xUnit's overload resolution falls through to Assert.Equal, producing CS1503. Use Assert.True with equality check instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index 34b628846519..0d0719717080 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -137,7 +137,7 @@ await InvokeOnMainThreadAsync(() => Assert.IsAssignableFrom(content.Layer.Mask); var mask = (CAShapeLayer)content.Layer.Mask!; mask.Dispose(); - Assert.Equal(IntPtr.Zero, mask.Handle); + Assert.True(mask.Handle == IntPtr.Zero, "Disposed mask should have a zeroed Handle"); // This should not throw ObjectDisposedException var ex = Record.Exception(() => content.RemoveFromSuperview()); From d83c425b80378a42ed7b3edd01df32eeac1e16d4 Mon Sep 17 00:00:00 2001 From: Timothy Sturm Date: Mon, 30 Mar 2026 10:56:09 -0500 Subject: [PATCH 5/7] Fix device test: clear native Layer.Mask reference before Dispose On real iOS/MacCatalyst devices, calling Dispose() on a CAShapeLayer does not zero its Handle while another native reference (Layer.Mask) still retains it. Clear the native reference first to properly simulate iOS deallocating the layer during view teardown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Handlers/ContentView/ContentViewTests.iOS.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index 0d0719717080..f49f21b0731e 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -130,16 +130,20 @@ await InvokeOnMainThreadAsync(() => contentView.Clip = new BorderStrokeStub(); contentView.LayoutSubviews(); - // Verify the mask was created, then dispose its native handle - // (simulating iOS deallocating the layer during view teardown). - // _contentMask is a StaticCAShapeLayer (subclass of CAShapeLayer), so use - // IsAssignableFrom rather than IsType to accept subclasses. + // Verify the mask was created Assert.IsAssignableFrom(content.Layer.Mask); var mask = (CAShapeLayer)content.Layer.Mask!; + + // Simulate iOS deallocating the layer during view teardown: + // 1. Clear the native reference so the retain count drops + // 2. Dispose the managed wrapper so Handle becomes IntPtr.Zero + // The ContentView's internal _contentMask field still references + // the disposed object, which is exactly the bug scenario. + content.Layer.Mask = null; mask.Dispose(); - Assert.True(mask.Handle == IntPtr.Zero, "Disposed mask should have a zeroed Handle"); - // This should not throw ObjectDisposedException + // This should not throw ObjectDisposedException — + // RemoveContentMask guards against disposed masks. var ex = Record.Exception(() => content.RemoveFromSuperview()); Assert.Null(ex); }); From ee4ad0864d67388e484f1412a1d76537a9e712a0 Mon Sep 17 00:00:00 2001 From: Timothy Sturm Date: Mon, 30 Mar 2026 10:57:13 -0500 Subject: [PATCH 6/7] Restore Handle assertion: clearing Layer.Mask ensures Dispose zeros it With content.Layer.Mask cleared before Dispose(), the native retain count drops to zero and Handle is properly zeroed. Re-add the assertion to verify the guard condition is actually being tested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index f49f21b0731e..ced7853e4ed0 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -141,9 +141,10 @@ await InvokeOnMainThreadAsync(() => // the disposed object, which is exactly the bug scenario. content.Layer.Mask = null; mask.Dispose(); + Assert.True(mask.Handle == IntPtr.Zero, "Disposed mask should have a zeroed Handle"); // This should not throw ObjectDisposedException — - // RemoveContentMask guards against disposed masks. + // RemoveContentMask guards against disposed masks via Handle check. var ex = Record.Exception(() => content.RemoveFromSuperview()); Assert.Null(ex); }); From 0dcd1816eb7cfdfa7fd0fde9b321bc5687511aaf Mon Sep 17 00:00:00 2001 From: Timothy Sturm Date: Mon, 30 Mar 2026 11:01:08 -0500 Subject: [PATCH 7/7] Use reflection for deterministic disposed-mask test Instead of relying on platform-specific Dispose() behavior (which varies depending on native retain counts), create a fresh CAShapeLayer with no native retains, Dispose it (deterministically zeroing Handle), then inject it into the private _contentMask field via reflection. This guarantees Handle == IntPtr.Zero on all platforms and directly tests the production guard in RemoveContentMask(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ContentView/ContentViewTests.iOS.cs | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs index ced7853e4ed0..ef0942cab1be 100644 --- a/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs +++ b/src/Core/tests/DeviceTests/Handlers/ContentView/ContentViewTests.iOS.cs @@ -1,4 +1,5 @@ using System; +using System.Reflection; using System.Threading.Tasks; using CoreAnimation; using CoreGraphics; @@ -132,19 +133,27 @@ await InvokeOnMainThreadAsync(() => // Verify the mask was created Assert.IsAssignableFrom(content.Layer.Mask); - var mask = (CAShapeLayer)content.Layer.Mask!; - - // Simulate iOS deallocating the layer during view teardown: - // 1. Clear the native reference so the retain count drops - // 2. Dispose the managed wrapper so Handle becomes IntPtr.Zero - // The ContentView's internal _contentMask field still references - // the disposed object, which is exactly the bug scenario. - content.Layer.Mask = null; - mask.Dispose(); - Assert.True(mask.Handle == IntPtr.Zero, "Disposed mask should have a zeroed Handle"); - - // This should not throw ObjectDisposedException — - // RemoveContentMask guards against disposed masks via Handle check. + + // Create a deterministically-disposed CAShapeLayer. + // A freshly-created layer with zero native retains is guaranteed + // to have Handle == IntPtr.Zero after Dispose(), regardless of + // platform-specific retain-count or GC timing behavior. + var disposedLayer = new CAShapeLayer(); + disposedLayer.Dispose(); + Assert.True(disposedLayer.Handle == IntPtr.Zero, "Disposed layer must have a zeroed Handle"); + + // Use reflection to inject the disposed layer into the private + // _contentMask field, simulating the race condition where iOS + // deallocates the native layer during view teardown while our + // managed field still holds a reference. + var field = typeof(Microsoft.Maui.Platform.ContentView) + .GetField("_contentMask", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(field); + field!.SetValue(contentView, disposedLayer); + + // RemoveFromSuperview triggers WillRemoveSubview → RemoveContentMask. + // Without the Handle guard, this would throw ObjectDisposedException + // when calling RemoveFromSuperLayer() on the disposed mask. var ex = Record.Exception(() => content.RemoveFromSuperview()); Assert.Null(ex); });