From 02ec4818bc32557debb4cffd0d3a4247ab73d7a3 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Fri, 23 Jan 2026 12:10:14 +0100 Subject: [PATCH] [bgen] Preserve all public methods supporting protocol methos as events. Fixes #24236. This has the same underlying cause as #24262, but the fix for #24262 was insufficient. The fix for #24262 ensured that if an event is listening for a protocol callback, then the corresponding protocol member implementation wouldn't be trimmed away. However, it didn't prevent protocol members from being trimmed if no event was listening for that protocol member to be called, which is a problem when the protocol member in question is a required protocol member, and native code would just call it without checking whether an implemented existed. For the bug in question, what happens is this: ```cs var mgr = new CBCentralManager (new DispatchQueue ("com.xamarin.tests.ios-plain")); // Uncomment to trigger bug, 'CBCentralManagerDelegate centralManagerDidUpdateState:' is a required protocol member // mgr.UpdatedState += (sender, e) => { // Console.WriteLine ("State: " + mgr.State); // }; mgr.DiscoveredPeripheral += (sender, e) => { }; mgr.ScanForPeripherals (); ``` In this case, there's an `ICBCentralManagerDelegate` instance assigned to `mgr.Delegate` (when an event handler was attached to the `DiscoveredPeripheral` event), but that `ICBCentralManagerDelegate` instance does not have an implementation for `ICBCentralManagerDelegate.UpdateState` (aka `CBCentralManagerDelegate centralManagerDidUpdateState:`), because it was trimmed away. This causes the bug when `CBCentralManager` calls its delegate's `centralManagerDidUpdateState:` selector. The fix is to always preserve all protocol members implemented by the internal class that implements the protocol for event handling purposes. Fixes https://github.com/dotnet/macios/issues/24236. --- src/bgen/Generator.cs | 9 ++++++-- tests/bindings-test/ApiDefinition.cs | 11 +++++++++ .../ObjCRuntime/RegistrarTest.cs | 23 ++++++++++++++++--- tests/test-libraries/libtest.h | 4 ++++ tests/test-libraries/libtest.m | 6 +++++ 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index df23c67caa12..a5e65796d80a 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -6720,6 +6720,13 @@ public void Generate (Type type) } else print ("public _{0} () {{ IsDirectBinding = false; }}\n", dtype.Name); + // Tell the trimmer to not remove any of our protocol implementations, they might be called from native + // code even if we don't have any event handlers listening for them. + print ($"[DynamicDependency (DynamicallyAccessedMemberTypes.PublicMethods, typeof (_{dtype.Name}))]"); + print ($"static _{dtype.Name} ()"); + print ("{"); + print ("\tGC.KeepAlive (null);"); // need to do _something_ (doesn't seem to matter what), otherwise the static cctor (and the DynamicDependency attributes) are trimmed away. + print ("}"); string shouldOverrideDelegateString = isProtocolEventBacked ? "" : "override "; @@ -6748,8 +6755,6 @@ public void Generate (Type type) } else previous_miname = miname; - // Tell the trimmer to not remove the delegate's method if someone is listening for the event - print ($"[DynamicDependency (nameof ({mi.Name}))]"); if (mi.ReturnType == TypeCache.System_Void) { if (bta.Singleton || mi.GetParameters ().Length == 1) print ("internal EventHandler? {0};", miname); diff --git a/tests/bindings-test/ApiDefinition.cs b/tests/bindings-test/ApiDefinition.cs index d050aa2ce2f0..0a998e249eec 100644 --- a/tests/bindings-test/ApiDefinition.cs +++ b/tests/bindings-test/ApiDefinition.cs @@ -853,6 +853,14 @@ interface HitchhikerDelegate { [EventArgs ("")] [Export ("buildIntergalacticHighway:")] void BuildIntergalacticHighway (Hitchhiker sender); + + [EventArgs ("")] + [Export ("byeByeDolphins:")] + void ByeByeDolphins (Hitchhiker sender); + + [EventArgs ("")] + [Export ("hitchhikeWithVogons:")] + void HitchhikeWithVogons (Hitchhiker sender); } [BaseType (typeof (NSObject), Delegates = new string [] { "Delegate" }, Events = new Type [] { typeof (HitchhikerDelegate) })] @@ -862,5 +870,8 @@ interface Hitchhiker { [Export ("destroyEarth")] void DestroyEarth (); + + [Export ("buildHighway")] + void BuildHighway (); } } diff --git a/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs b/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs index e32530fd11e0..0df5e8bd75b3 100644 --- a/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs +++ b/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs @@ -21,6 +21,7 @@ using PlatformException = ObjCRuntime.RuntimeException; using NativeException = ObjCRuntime.ObjCException; using CoreAnimation; +using CoreBluetooth; using CoreGraphics; using CoreLocation; #if !__TVOS__ @@ -56,15 +57,31 @@ public static Registrars CurrentRegistrar { [Test] public void EventTestCustomType () { - var earthDestroyed = false; + var earthDestroyed = 0; using var vogons = new Hitchhiker (); vogons.BuildIntergalacticHighway += (object sender, EventArgs ea) => { - earthDestroyed = true; + earthDestroyed++; }; vogons.DestroyEarth (); - Assert.That (earthDestroyed, Is.True, "Event raised"); + Assert.That (earthDestroyed, Is.EqualTo (1), "Event raised"); + + vogons.BuildHighway (); + Assert.That (earthDestroyed, Is.EqualTo (2), "Event raised"); } + [Test] + public void EventTestOSType () + { + using var mgr = new CBCentralManager (new DispatchQueue ("com.xamarin.tests.ios-plain")); + // The bug happens when there's no event listener for 'UpdatedState', which is a required protocol method. + // mgr.UpdatedState += (sender, e) => { }; + + // attach at least one event handler, to create an instance of the internal class that transforms protocol callbacks into events + mgr.DiscoveredPeripheral += (sender, e) => { }; + + // mimic what CBCentralManager does, instead of having to spin up the entire Bluetooth stack. + Messaging.void_objc_msgSend_IntPtr (mgr.Delegate.Handle, Selector.GetHandle ("centralManagerDidUpdateState:"), IntPtr.Zero); + } [Test] public void EventTestSdkType () diff --git a/tests/test-libraries/libtest.h b/tests/test-libraries/libtest.h index ba0149a7a06b..1797a6379b7b 100644 --- a/tests/test-libraries/libtest.h +++ b/tests/test-libraries/libtest.h @@ -367,14 +367,18 @@ typedef void (^outerBlock) (innerBlock callback); @class Hitchhiker; @protocol HitchhikerDelegate +@required + -(void) byeByeDolphins: (Hitchhiker *) sender; @optional -(void) buildIntergalacticHighway: (Hitchhiker *) sender; + -(void) hitchhikeWithVogons: (Hitchhiker *) sender; @end @interface Hitchhiker : NSObject { } @property (retain) id delegate; -(void) destroyEarth; +-(void) buildHighway; @end #pragma clang diagnostic pop diff --git a/tests/test-libraries/libtest.m b/tests/test-libraries/libtest.m index 16c7cb33def1..19807b9bf5cd 100644 --- a/tests/test-libraries/libtest.m +++ b/tests/test-libraries/libtest.m @@ -1542,6 +1542,12 @@ @implementation ClassWithNoDefaultCtor : NSObject { @implementation Hitchhiker : NSObject { } -(void) destroyEarth +{ + [self.delegate byeByeDolphins: self]; + [self.delegate buildIntergalacticHighway: self]; + [self.delegate hitchhikeWithVogons: self]; +} +-(void) buildHighway { [self.delegate buildIntergalacticHighway: self]; }