Skip to content

Commit 0062608

Browse files
committed
OSX/MACOS: Make [CocoaView get] honour the +0 "get" convention
+[CocoaView get] is a cached-singleton accessor. By Cocoa convention a method whose name does not start with new / alloc / copy / mutableCopy returns a +0 (unretained) reference; the singleton is kept alive by the storage it is cached in. The existing implementation violated that convention on the first-time path: it did [CocoaView new] (+1) and handed the pointer to nsview_set_ptr, which was a raw `g_instance = p;` - no retain. The caller then received a +1 return on the first call but +0 on every call thereafter. Under ARC the strong static would retain on store so it was self-consistent, but under MRR the first-ever caller had an unbalanced +1 the code never handled - a small process-lifetime leak. A prior commit (e9279a8) leaned into the +0 assumption by adding RARCH_RETAIN wrappers in ui_cocoa.m, which silently turned that +1 into a +2 leak whenever -setViewType: was called for a singleton- backed view type on its first use. Two changes, both behind the existing RARCH_* macros so ARC builds see no-ops: - +[CocoaView get]: autorelease the freshly-allocated instance before storing. The returned pointer is now +0 on every path, matching the get naming convention. - nsview_set_ptr: take a proper strong reference for the g_instance slot (retain new, release old). g_instance is now the single owner of the singleton's +1, matching how the rest of the codebase already treats it (never nil-ed, never explicitly released - so the retain lives for the process lifetime, which is the intent). Adds #include <defines/cocoa_defines.h> to cocoa_common.m for RARCH_RETAIN / RARCH_RELEASE / RARCH_AUTORELEASE. The include path is already on every Apple build config (already used by ui_cocoa.m, dispserv_apple.m, coreaudio.c). No behavioural change under ARC. On MRR, plugs the first-call +1 leak and cancels the +2 over-retain that e9279a8's RARCH_RETAIN wrappers would otherwise produce.
1 parent e0b7b9b commit 0062608

1 file changed

Lines changed: 31 additions & 2 deletions

File tree

ui/drivers/cocoa/cocoa_common.m

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include <compat/apple_compat.h>
2121
#include <string/stdstring.h>
22+
#include <defines/cocoa_defines.h>
2223

2324
#include "cocoa_common.h"
2425
#include "apple_platform.h"
@@ -206,7 +207,17 @@ + (CocoaView*)get
206207
CocoaView *view = (BRIDGE CocoaView*)nsview_get_ptr();
207208
if (!view)
208209
{
209-
view = [CocoaView new];
210+
/* +new returns +1 owned by the caller. Autorelease before
211+
* handing to nsview_set_ptr, which takes its own retain for
212+
* the process-lifetime g_instance slot. Net result: the
213+
* returned pointer obeys the Cocoa +0 "get" convention on
214+
* both the first call (where we allocate) and every
215+
* subsequent call (where we just read g_instance) - so
216+
* callers do not have to guess the retain count. Under ARC
217+
* RARCH_AUTORELEASE is a no-op; the strong local's end-of-
218+
* scope release balances +new's +1 after g_instance's
219+
* storeStrong has taken its own retain. */
220+
view = RARCH_AUTORELEASE([CocoaView new]);
210221
nsview_set_ptr(view);
211222
#if defined(IOS)
212223
view.displayLink = [CADisplayLink displayLinkWithTarget:view selector:@selector(step:)];
@@ -936,7 +947,25 @@ float cocoa_screen_get_native_scale(void)
936947
return (BRIDGE void *)g_instance;
937948
}
938949

939-
void nsview_set_ptr(CocoaView *p) { g_instance = p; }
950+
void nsview_set_ptr(CocoaView *p)
951+
{
952+
/* g_instance is the process-lifetime strong reference to the
953+
* CocoaView singleton. Under MRR we must explicitly retain the
954+
* new value and release the old one so g_instance owns a
955+
* balanced +1 across reassignments. In practice there is only
956+
* one caller (+[CocoaView get] on its first-time path), but the
957+
* invariant matters: callers treat [CocoaView get] as a +0 "get"
958+
* accessor, and that only holds if g_instance is the one keeping
959+
* the view alive. Under ARC RARCH_RETAIN and RARCH_RELEASE are
960+
* no-ops; the static __strong pointer does retain/release via
961+
* objc_storeStrong when assigned. */
962+
if (g_instance != p)
963+
{
964+
RARCH_RETAIN(p);
965+
RARCH_RELEASE(g_instance);
966+
g_instance = p;
967+
}
968+
}
940969

941970
CocoaView *cocoaview_get(void)
942971
{

0 commit comments

Comments
 (0)