OSX/MACOS/COCOATOUCH: Normalise MRR safety in ui_cocoatouch.m

LibretroAdmin · LibretroAdmin · commit 4c5f3c34896c · 2026-04-21T19:47:32.000+02:00
Applies the same treatment to ui_cocoatouch.m that e9279a8 and bd45086 applied to ui_cocoa.m / cocoa_gl_ctx.m: make the file correct under both ARC and MRR via the RARCH_*/#if __has_feature(objc_arc) dance the rest of the Cocoa code uses. Every shipping iOS Xcode project sets CLANG_ENABLE_OBJC_ARC=YES, so none of the MRR leaks listed below bite any current build; this is future-proofing / consistency with ui_cocoa.m rather than bug-fixing today. The one thing that does change in practice is that the file now compiles under MRR at all - the previous (nonatomic, strong) on keyboardTextField was an ARC-only keyword that failed to parse under the pre-ARC compiler. Header changes (apple_platform.h): The RetroArch_iOS properties were spelled with bare (nonatomic), relying on ARC's implicit 'strong' default for object-typed properties. Under MRR the default is 'assign', which is silently wrong - the referent is released at the next autorelease-pool drain after assignment. Spell the ownership out: window and bgDate become (nonatomic, retain), documentsDirectory becomes (nonatomic, copy) to match the NSString convention. menu_count (int) stays bare (nonatomic) since 'assign' is correct for scalars. Under ARC these qualifiers are accepted and behave identically to 'strong' / defaults. Implementation changes (ui_cocoatouch.m): - Include <defines/cocoa_defines.h> for the RARCH_* macros. - The private category's (nonatomic, strong) UITextField *keyboardTextField becomes (nonatomic, retain). Identical behaviour under ARC; parseable under MRR. - -setViewType: adopts the +1-invariant pattern e9279a8 introduced for ui_cocoa.m. _renderView is released via RARCH_RELEASE before the ivar is nilled on teardown; the VULKAN and METAL branches take their +1 directly from +new; the OPENGL_ES branch takes it via RARCH_RETAIN of the glkitview_init() singleton so the ivar's ownership is uniform across all three paths. UIPointerInteraction's alloc+init is autoreleased to balance -addInteraction:'s internal retain. - Every other local +1 producer (+[KSCrashConfiguration new], +[KSCrashReportStoreConfiguration new], -mutableCopy, [[NSString alloc] initWithData:...] in three places, [[NSDateFormatter alloc] init], [[UIWindow alloc] initWithFrame:...], [[NSURLComponents alloc] initWithURL:...], [[UITextField alloc] initWithFrame:...]) is paired with a RARCH_AUTORELEASE on the following line. The two sites that assign to retain properties (self.window and self.keyboardTextField) use an explicit temp so the alloc can be autoreleased before the setter takes its own retain - this is the classic pre-ARC [[[Class alloc] init...] autorelease] -> self.prop = ... idiom, spelled out across three statements so the macro only ever appears in statement form. (RARCH_AUTORELEASE expands to ((void)0) under ARC - it is statement-only, not expression-form. See aed4543.) - Adds an MRR-only -dealloc that RARCH_RELEASEs every retained ivar / property and calls RARCH_SUPER_DEALLOC(), mirroring ui_cocoa.m. RetroArch_iOS is the UIApplication delegate so this dealloc effectively never runs in practice, but it completes the ownership picture and keeps both files in step. cocoa_vk_ctx.m was already clean (audited in the same pass as bd45086); no changes there.
diff --git a/ui/drivers/cocoa/apple_platform.h b/ui/drivers/cocoa/apple_platform.h
@@ -102,10 +102,17 @@ UINavigationControllerDelegate> {
     apple_view_type_t _vt;
 }
 
-@property (nonatomic) UIWindow* window;
-@property (nonatomic) NSString* documentsDirectory;
-@property (nonatomic) int menu_count;
-@property (nonatomic) NSDate *bgDate;
+/* Explicit retain / copy qualifiers so these properties are correct
+ * under MRR as well as ARC.  Under ARC the object-typed property
+ * default is 'strong', which behaves identically to 'retain' here;
+ * under MRR the default is 'assign', which would silently drop the
+ * retain and release the referent at the next autorelease pool
+ * drain.  Spelling the ownership out keeps ui_cocoatouch.m
+ * MRR-buildable in the same spirit as ui_cocoa.m. */
+@property (nonatomic, retain) UIWindow *window;
+@property (nonatomic, copy)   NSString *documentsDirectory;
+@property (nonatomic)         int menu_count;
+@property (nonatomic, retain) NSDate *bgDate;
 
 + (RetroArch_iOS*)get;
 
diff --git a/ui/drivers/ui_cocoatouch.m b/ui/drivers/ui_cocoatouch.m
@@ -24,6 +24,7 @@
 #include <queues/task_queue.h>
 #include <string/stdstring.h>
 #include <retro_timers.h>
+#include <defines/cocoa_defines.h>
 
 #include "cocoa/cocoa_common.h"
 #include "cocoa/apple_platform.h"
@@ -537,7 +538,10 @@ @interface RetroArch_iOS () <MXMetricManagerSubscriber, UIPointerInteractionDele
 #endif
 
 @interface RetroArch_iOS () <UITextFieldDelegate>
-@property (nonatomic, strong) UITextField *keyboardTextField;
+/* 'retain' works identically to 'strong' under ARC but unlike 'strong'
+ * is also accepted by the pre-ARC compiler - so the file remains
+ * buildable under MRR without a separate code path. */
+@property (nonatomic, retain) UITextField *keyboardTextField;
 @property (nonatomic, copy) void(^keyboardCompletionCallback)(const char *);
 @property (nonatomic, assign) char **keyboardBufferPtr;
 @property (nonatomic, assign) size_t *keyboardSizePtr;
@@ -563,13 +567,24 @@ - (void)setViewType:(apple_view_type_t)vt
    if (_renderView != nil)
    {
       [_renderView removeFromSuperview];
+      /* _renderView holds a +1 retain regardless of which path below
+       * created it (the Metal / Vulkan branches take +1 directly from
+       * +new; the OPENGL_ES branch retains the singleton returned by
+       * glkitview_init()).  Release it here so the ownership invariant
+       * is balanced before we nil the ivar.  Under ARC this is a
+       * no-op and the implicit __strong ivar handles the release when
+       * _renderView is assigned nil. */
+      RARCH_RELEASE(_renderView);
       _renderView = nil;
    }
 
    switch (vt)
    {
 #ifdef HAVE_COCOA_METAL
        case APPLE_VIEW_TYPE_VULKAN:
+         /* +new returns a +1 object; that retain transfers into
+          * _renderView and satisfies the ivar's ownership invariant
+          * directly.  No extra RARCH_RETAIN needed. */
          _renderView = [MetalLayerView new];
 #if TARGET_OS_IOS
          _renderView.multipleTouchEnabled = YES;
@@ -588,7 +603,12 @@ - (void)setViewType:(apple_view_type_t)vt
          break;
 #endif
        case APPLE_VIEW_TYPE_OPENGL_ES:
-         _renderView = (BRIDGE GLKView*)glkitview_init();
+         /* glkitview_init() returns an unretained pointer to the
+          * cocoa_gl_ctx.m singleton.  Retain explicitly so _renderView
+          * matches the +1 invariant the Metal / Vulkan paths get from
+          * +new.  Under ARC RARCH_RETAIN is a no-op and the implicit
+          * __strong ivar assignment takes the retain via objc_storeStrong. */
+         _renderView = RARCH_RETAIN((BRIDGE GLKView*)glkitview_init());
          break;
 
        case APPLE_VIEW_TYPE_NONE:
@@ -602,7 +622,16 @@ - (void)setViewType:(apple_view_type_t)vt
 #if TARGET_OS_IOS
    if (@available(iOS 13.4, *))
    {
-      [_renderView addInteraction:[[UIPointerInteraction alloc] initWithDelegate:self]];
+      /* +[UIPointerInteraction alloc] initWithDelegate: returns +1.
+       * -addInteraction: retains internally, so autorelease our own
+       * +1 to balance under MRR.  ARC already releases on scope
+       * exit; the macro is a no-op there.  RARCH_AUTORELEASE is a
+       * statement-only macro (it expands to ((void)0) under ARC)
+       * so it must appear on its own line rather than wrapping the
+       * rvalue. */
+      UIPointerInteraction *interaction = [[UIPointerInteraction alloc] initWithDelegate:self];
+      RARCH_AUTORELEASE(interaction);
+      [_renderView addInteraction:interaction];
       _renderView.userInteractionEnabled = YES;
    }
 #endif
@@ -700,10 +729,18 @@ - (void)initKSCrash
       }
    }
 
-   /* Configure KSCrash for local storage only */
+   /* Configure KSCrash for local storage only.
+    * Autorelease the +1 from +new: -installWithConfiguration: keeps
+    * its own reference via config.reportStoreConfiguration retain and
+    * KSCrash's own retain of the config, so our local can be released
+    * at autorelease-pool drain without dangling any of those.
+    * RARCH_AUTORELEASE is a statement-only macro; call it on its own
+    * line after the assignment.  No-op under ARC. */
    KSCrashConfiguration *config = [KSCrashConfiguration new];
+   RARCH_AUTORELEASE(config);
    config.installPath = crashReportsPath;
    KSCrashReportStoreConfiguration *storeConfig = [KSCrashReportStoreConfiguration new];
+   RARCH_AUTORELEASE(storeConfig);
    storeConfig.reportsPath = crashReportsPath;
    storeConfig.appName = @"RetroArch";
    storeConfig.maxReportCount = 10; /* Keep last 10 crash reports */
@@ -751,7 +788,12 @@ - (void)processKSCrashReports
       if (!report)
          continue;
 
+      /* -mutableCopy returns +1.  Inside a for-loop that's a
+       * per-iteration leak under MRR; autorelease so it is cleaned up
+       * when the pool drains at the next run-loop iteration.
+       * Statement-only macro, so on its own line.  No-op under ARC. */
       NSMutableDictionary *mutableReport = [report.value mutableCopy];
+      RARCH_AUTORELEASE(mutableReport);
 
       /* Remove binary_images to reduce file size */
       if ([mutableReport objectForKey:@"binary_images"])
@@ -776,8 +818,12 @@ - (void)processKSCrashReports
                                                                error:nil];
       if (minifiedData)
       {
+         /* +1 from alloc+init; per-iteration leak inside the for-loop
+          * under MRR without an autorelease.  Statement-only macro, so
+          * on its own line.  No-op under ARC. */
          NSString *jsonString = [[NSString alloc] initWithData:minifiedData
                                                       encoding:NSUTF8StringEncoding];
+         RARCH_AUTORELEASE(jsonString);
          if (jsonString)
          {
             /* Log with a unique marker that can be extracted with grep/sed */
@@ -817,7 +863,11 @@ - (void)applicationDidFinishLaunching:(UIApplication *)application
 
       // Define the original and new file paths
       NSString *originalPath = [cachesDirectory stringByAppendingPathComponent:@"RetroArch/config/retroarch.cfg"];
+      /* +1 from alloc+init; autorelease so scope-exit cleans up under
+       * MRR the same way ARC does.  Statement-only macro, so on its
+       * own line.  No-op under ARC. */
       NSDateFormatter *dateFormatter = [[NSDateFormatter alloc] init];
+      RARCH_AUTORELEASE(dateFormatter);
       [dateFormatter setDateFormat:@"HHmm-yyMMdd"];
       NSString *timestamp = [dateFormatter stringFromDate:[NSDate date]];
       NSString *newPath = [cachesDirectory stringByAppendingPathComponent:[NSString stringWithFormat:@"RetroArch/config/RetroArch-%@.cfg", timestamp]];
@@ -840,8 +890,16 @@ - (void)applicationDidFinishLaunching:(UIApplication *)application
 
    [self setDelegate:self];
 
-   /* Setup window */
-   self.window        = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
+   /* Setup window.
+    * self.window is a retain property (see apple_platform.h); the
+    * setter takes its own retain.  Autorelease the +1 from alloc+init
+    * via a temp so the setter's retain is the sole owner under MRR.
+    * Under ARC the strong setter retains and ARC scope-releases the
+    * temp.  Statement-only macro, so RARCH_AUTORELEASE goes on its
+    * own line. */
+   UIWindow *win      = [[UIWindow alloc] initWithFrame:[[UIScreen mainScreen] bounds]];
+   RARCH_AUTORELEASE(win);
+   self.window        = win;
    [self.window makeKeyAndVisible];
 
    [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(handleAudioSessionInterruption:) name:AVAudioSessionInterruptionNotification object:[AVAudioSession sharedInstance]];
@@ -1056,7 +1114,11 @@ -(BOOL)openRetroArchURL:(NSURL *)url
    // Handle topshelf URLs: retroarch://topshelf?path=...&core_path=...
    if ([url.host isEqualToString:@"topshelf"])
    {
+      /* +1 from alloc+init; autorelease so scope-exit balances under
+       * MRR.  Statement-only macro, so on its own line.  No-op under
+       * ARC. */
       NSURLComponents *comp = [[NSURLComponents alloc] initWithURL:url resolvingAgainstBaseURL:NO];
+      RARCH_AUTORELEASE(comp);
       NSString *ns_path, *ns_core_path;
       char path[PATH_MAX_LENGTH];
       char core_path[PATH_MAX_LENGTH];
@@ -1152,7 +1214,14 @@ - (void)showGameView
    /* Initialize hidden keyboard text field for iOS native keyboard support */
    if (!self.keyboardTextField)
    {
-      self.keyboardTextField = [[UITextField alloc] initWithFrame:CGRectMake(0, -100, 1, 1)];
+      /* self.keyboardTextField is a retain property (see private
+       * category above); the setter takes its own retain.  Autorelease
+       * the +1 from alloc+init via a temp so the setter's retain is
+       * the sole owner under MRR.  Statement-only macro, so
+       * RARCH_AUTORELEASE goes on its own line.  No-op under ARC. */
+      UITextField *tf = [[UITextField alloc] initWithFrame:CGRectMake(0, -100, 1, 1)];
+      RARCH_AUTORELEASE(tf);
+      self.keyboardTextField = tf;
       self.keyboardTextField.delegate = self;
       self.keyboardTextField.autocapitalizationType = UITextAutocapitalizationTypeNone;
       self.keyboardTextField.autocorrectionType = UITextAutocorrectionTypeNo;
@@ -1176,7 +1245,11 @@ - (void)didReceiveMetricPayloads:(NSArray<MXMetricPayload *> *)payloads API_AVAI
 {
     for (MXMetricPayload *payload in payloads)
     {
+        /* +1 from alloc+init; per-iteration leak inside the loop under
+         * MRR without an autorelease.  Statement-only macro, so on its
+         * own line.  No-op under ARC. */
         NSString *json = [[NSString alloc] initWithData:[payload JSONRepresentation] encoding:kCFStringEncodingUTF8];
+        RARCH_AUTORELEASE(json);
         RARCH_LOG("[Cocoa] Got Metric Payload:\n%s\n", [json cStringUsingEncoding:kCFStringEncodingUTF8]);
     }
 }
@@ -1185,7 +1258,11 @@ - (void)didReceiveDiagnosticPayloads:(NSArray<MXDiagnosticPayload *> *)payloads
 {
     for (MXDiagnosticPayload *payload in payloads)
     {
+        /* +1 from alloc+init; per-iteration leak inside the loop under
+         * MRR without an autorelease.  Statement-only macro, so on its
+         * own line.  No-op under ARC. */
         NSString *json = [[NSString alloc] initWithData:[payload JSONRepresentation] encoding:kCFStringEncodingUTF8];
+        RARCH_AUTORELEASE(json);
         RARCH_LOG("[Cocoa] Got Diagnostic Payload:\n%s\n", [json cStringUsingEncoding:kCFStringEncodingUTF8]);
     }
 }
@@ -1314,6 +1391,27 @@ - (void)textFieldDidEndEditing:(UITextField *)textField
    }
 }
 
+#if !__has_feature(objc_arc)
+/* RetroArch_iOS is the UIApplication delegate and therefore a
+ * process-lifetime singleton - this dealloc effectively never runs in
+ * practice.  Keeping it for symmetry with ui_cocoa.m's dealloc and so
+ * the ownership picture is complete for anyone reading the file: every
+ * retained ivar / property has a paired release here, and _renderView
+ * is released by -setViewType: whenever it is reassigned (see above).
+ * No-op under ARC where retained ivars/properties are released
+ * automatically. */
+- (void)dealloc
+{
+   RARCH_RELEASE(_renderView);
+   RARCH_RELEASE(_window);
+   RARCH_RELEASE(_documentsDirectory);
+   RARCH_RELEASE(_bgDate);
+   RARCH_RELEASE(_keyboardTextField);
+   RARCH_RELEASE(_keyboardCompletionCallback);
+   RARCH_SUPER_DEALLOC();
+}
+#endif
+
 @end
 
 ui_companion_driver_t ui_companion_cocoatouch = {
