[net10.0] [UIKit] Fix a GC race in UIBarButtonItem. #21572
Conversation
Rewrite how we handle events/callbacks for UIBarButtonItem: there's no need
for a special class to receive the click events, we can use the same
UIBarButtonItem instance.
This simplifies the code a lot, and should also fix this random exception:
Failed to marshal the Objective-C object 0x2800fcc00 (type: UIKit_UIBarButtonItem_Callback). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance (because the type 'UIKit.UIBarButtonItem+Callback' does not have a constructor that takes one NativeHandle argument).
at ObjCRuntime.Runtime.MissingCtor(IntPtr, IntPtr, Type, Runtime.MissingCtorResolution, IntPtr, RuntimeMethodHandle) + 0x1dc
at ObjCRuntime.Runtime.ConstructNSObject[T](IntPtr, Type, Runtime.MissingCtorResolution, IntPtr, RuntimeMethodHandle) + 0xfc
at ObjCRuntime.Runtime.ConstructNSObject(IntPtr, IntPtr, Runtime.MissingCtorResolution) + 0x74
at UIKit.UIBarButtonItem.get_Target() + 0x64
at UIKit.UIBarButtonItem..ctor(UIBarButtonSystemItem, NSObject, Selector) + 0x94
at UIKit.UIBarButtonItem..ctor(UIBarButtonSystemItem, EventHandler) + 0x90
This happens because we did something equivalent to this:
public UIBarButtonItem (UIBarButtonSystemItem systemItem, EventHandler handler)
: this (systemItem, new Callback (), actionSel)
{
instanceField = (Callback) Target;
}
The problem is that the Target property on the native UIBarButtonItem is
defined with ArgumentSemantic.Assign, so it won't be retained. This means that
if the GC runs between the creation of the native UIBarButtonItem, and
fetching the Target property to assign it to the instance field later on in
the managed constructor, the GC is free to collect the managed Callback
instance, and then, when the code tries to fetch the Target property, the
exception occurs.
This was reported on Discord (thanks @tipa!): https://discord.com/channels/732297728826277939/732297808148824115/1301578382248509552
The partial-static registrar is still also partially dynamic.
💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
✅ API diff for current PR / commit.NET (No breaking changes)✅ API diff vs stable.NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 101 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
@rolfbjarne is there by any chance a simple workaround that could resolve the crash until .net10 is released? I don't know if it's because of Xcode 16 and the new tooling, or because I recently refactored my ViewController initialization logic or because I am using a new way to log my crash reports, but I am seeing this crash more frequently in the last few days. |
|
@tipa I haven't tried this, but subclassing public class MyUIBarButtonItem : UIBarButtonItem {
const string MySelector = "myUIBarButtonItemCallback:";
static Selector actionSel = new Selector (MySelector);
public MyUIBarButtonItem (UIImage image, UIBarButtonItemStyle style, EventHandler handler)
: base (image, style, this, actionSel))
{
}
[Export (MySelector)]
void Call (NSObject sender)
{
OnClicked (sender);
}
EventHandler? my_clicked;
internal void OnClicked (NSObject sender)
{
if (my_clicked is not null)
my_clicked (sender, EventArgs.Empty);
}
public event EventHandler MyClicked {
add {
if (my_clicked is null) {
Target = this;
this.Action = actionSel;
MarkDirty ();
}
my_clicked += value;
}
remove {
my_clicked -= value;
}
}
} |
|
@rolfbjarne thanks for that workaround, could have thought of that myself... :D Should I also call Thanks again! |
Rewrite how we handle events/callbacks for UIBarButtonItem: there's no need for a special class to receive the click events, we can use the same UIBarButtonItem instance through a category.
This simplifies the code a lot, and should also fix this random exception:
This happens because we did something equivalent to this:
The problem is that the Target property on the native UIBarButtonItem (which is set from the second argument to the base constructor, the "new Callback ()" code) is defined with ArgumentSemantic.Assign, so it won't be retained. This means that if the GC runs between the creation of the native UIBarButtonItem, and fetching the Target property to assign it to the instance field later on in the managed constructor, the GC is free to collect the managed Callback instance, and then, when the code tries to fetch the Target property, the exception occurs because the managed instance has already been collected.
This was reported on Discord (thanks @tipa!): https://discord.com/channels/732297728826277939/732297808148824115/1301578382248509552
Also a few other fixes:
[DynamicDependencyAttribute]instead of[Preserve].[Extension]attribute unless we're using a static registrar, because the dynamic registrar needs it.