From ce3501698301023407da48f0839e3d150289bd27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linn=20Kristin=20Bj=C3=B8rkli?= Date: Wed, 12 Aug 2026 01:57:42 +0200 Subject: [PATCH] [UIKit] Protect RegisterForTraitChanges observers from premature GC toggle-ref collection RegisterForTraitChanges (and its NSObject-target/Selector overloads) hand a reference to the observing object to UIKitCore's private _UITraitChangeRegistry, but never call MarkDirty() to register that object with the Mono toggle-ref GC bridge. Every other API in this codebase that hands a reference to native code for later callback (UIControl.AddTarget, UIGestureRecognizer, NSNotificationCenter.AddObserver, UIBarButtonItem, UIPickerView) already does this. Without it, xamarin_gc_toggleref_callback falls back to inferring liveness from -retainCount, which only reflects normal ObjC retains. Observer registries are conventionally non-retaining (to avoid retain cycles), so _UITraitChangeRegistry holding a reference to an object doesn't show up in its retainCount. If retainCount == 1 at the next GC, the bridge downgrades the managed peer to a weak GC handle, and it can be collected while _UITraitChangeRegistry still holds a now-dangling pointer to it - corrupting a registry that is touched by many unrelated UIKit code paths afterwards. We've observed this manifest as SIGSEGV/EXC_BAD_ACCESS crashes inside _UITraitChangeRegistry across a wide variety of unrelated call sites (UICollectionView teardown, gesture node updates, ScrollEdgeEffectView, UITextField construction) on iOS 26, none of which touch RegisterForTraitChanges themselves - consistent with heap corruption surfacing far from its actual cause. This patch marks the observer (and, for the target/action overload, the target) dirty before registration, mirroring the established pattern elsewhere in this codebase. Not able to validate this compiles/passes CI locally (this checkout could not resolve the pinned internal preview SDK/runtime packages from public NuGet feeds) - opening as a draft for CI + maintainer review. --- src/UIKit/UITraitChangeObservable.cs | 31 ++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/UIKit/UITraitChangeObservable.cs b/src/UIKit/UITraitChangeObservable.cs index 156e35ccc907..ec20e57aca95 100644 --- a/src/UIKit/UITraitChangeObservable.cs +++ b/src/UIKit/UITraitChangeObservable.cs @@ -26,6 +26,26 @@ public static Class [] ToClasses (params Type [] traits) return Class.FromTypes (traits); } + // Register the observing object with the toggle-ref GC bridge before we hand a + // reference to it to native code. Without this, the managed peer's toggle-ref status + // defaults to whatever xamarin_gc_toggleref_callback infers from -retainCount, which + // only reflects normal ObjC retains: it can't see that _UITraitChangeRegistry now also + // holds a reference to this object internally (observer registries are conventionally + // non-retaining, to avoid retain cycles with their observers). If -retainCount is 1 at + // the next GC, the bridge downgrades the peer to a weak GC handle and it can be + // collected while _UITraitChangeRegistry still references it, corrupting the shared + // registry (a crash then tends to surface later, in unrelated code that next touches + // the registry, rather than here). MarkDirty is idempotent and mirrors the pattern + // already used by UIControl.AddTarget, UIGestureRecognizer, and + // NSNotificationCenter.AddObserver for the same reason. + private static void MarkDirtyForTraitRegistration (IUITraitChangeObservable observable) + { + // NSObject.MarkDirty() is 'protected'; the (bool) overload is 'internal' and can be + // called from anywhere in this assembly, which is what we need from a static method + // on an unrelated interface. + (observable as NSObject)?.MarkDirty (false); + } + /// /// Registers a callback handler that will be executed when one of the specified traits changes. /// @@ -39,6 +59,7 @@ public IUITraitChangeRegistration RegisterForTraitChanges (Type [] traits, Actio internal static IUITraitChangeRegistration _RegisterForTraitChanges (IUITraitChangeObservable This, Type [] traits, Action handler) { + MarkDirtyForTraitRegistration (This); return _RegisterForTraitChanges (This, ToClasses (traits), handler); } @@ -56,6 +77,7 @@ public IUITraitChangeRegistration RegisterForTraitChanges (Action handler, params Type [] traits) { // Add an override with 'params', unfortunately this means reordering the parameters. + MarkDirtyForTraitRegistration (This); return _RegisterForTraitChanges (This, ToClasses (traits), handler); } @@ -74,6 +96,7 @@ public IUITraitChangeRegistration RegisterForTraitChanges (Action (IUITraitChangeObservable This, Action handler) where T : IUITraitDefinition { + MarkDirtyForTraitRegistration (This); return _RegisterForTraitChanges (This, ToClasses (typeof (T)), handler); } @@ -95,6 +118,7 @@ internal static IUITraitChangeRegistration _RegisterForTraitChanges (IUI where T1 : IUITraitDefinition where T2 : IUITraitDefinition { + MarkDirtyForTraitRegistration (This); return _RegisterForTraitChanges (This, ToClasses (typeof (T1), typeof (T2)), handler); } @@ -119,6 +143,7 @@ internal static IUITraitChangeRegistration _RegisterForTraitChanges where T2 : IUITraitDefinition where T3 : IUITraitDefinition { + MarkDirtyForTraitRegistration (This); return _RegisterForTraitChanges (This, ToClasses (typeof (T1), typeof (T2), typeof (T3)), handler); } @@ -146,6 +171,7 @@ internal static IUITraitChangeRegistration _RegisterForTraitChanges