Skip to content

Commit bd45086

Browse files
committed
OSX/MACOS/COCOATOUCH: Normalise MRR safety in cocoa_gl_ctx.m
Audit of the OpenGL context driver turned up three latent MRR leaks, all on the iOS/tvOS side where the code relies on ARC's strong-store semantics for release-on-reassignment. Every shipping iOS build currently enables ARC, so none of these leak in practice today; making the paths MRR-safe as well brings the driver in line with cocoa_common.m / ui_cocoa.m (which compile under both ARC and MRR via the RARCH_*/RELEASE macro family) and keeps the qb/make-style non-Xcode build viable for COCOATOUCH targets. All three fixes use the existing RELEASE macro from cocoa_common.h, which under ARC expands to plain `x = nil` and under MRR to `[x release]; x = nil` - so this is a no-op under ARC and a strict correctness improvement under MRR. - cocoa_gl_gfx_ctx_destroy: previously only the OSX branch called RELEASE on g_ctx / g_hw_ctx; the iOS branch dropped the +1 from [[EAGLContext alloc] initWithAPI:...] with a raw `g_ctx = nil`. Move the RELEASE calls unconditional (post-#endif) and add RELEASE(glk_view) under #if defined(HAVE_COCOATOUCH) so the GLKView created in glkitview_init does not leak across init/destroy cycles even on clean shutdown. - cocoa_gl_gfx_ctx_set_video_mode (both branches): add defensive RELEASE(g_ctx) / RELEASE(g_hw_ctx) before the [[NSOpenGLContext alloc] ...] / [[EAGLContext alloc] ...] overwrites. The normal reinit flow has -destroy running first so both statics are already nil here, but guarding defensively means a future re-init-without-destroy call site cannot leak the previous +1. - glkitview_init: release the previous glk_view before overwriting with [GLKView new]. The existing [glk_view deleteDrawable] handled the GL-side cleanup but left the Obj-C retain dangling under MRR. cocoa_vk_ctx.m has no static Obj-C pointers and needed no changes. MoltenVK / vulkan_context_destroy own the Vulkan surface / device lifetime, and all CocoaView accesses there are brief unretained reads backed by cocoa_common.m's g_instance strong reference (see 0062608).
1 parent 0062608 commit bd45086

1 file changed

Lines changed: 34 additions & 4 deletions

File tree

gfx/drivers_context/cocoa_gl_ctx.m

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,11 @@ void cocoa_gl_gfx_ctx_update(void)
144144
* glPushGroupMarkerEXT with no current context and crashes. */
145145
if (glk_view)
146146
[glk_view deleteDrawable];
147+
/* RELEASE the old view (+1 from the previous [GLKView new]) before
148+
* overwriting the static. Under ARC the strong static would retain/
149+
* release on store, but under MRR the raw assignment below would drop
150+
* the +1 and leak one GLKView per re-init. */
151+
RELEASE(glk_view);
147152

148153
glk_view = [GLKView new];
149154
#if TARGET_OS_IOS
@@ -172,10 +177,8 @@ static void cocoa_gl_gfx_ctx_destroy(void *data)
172177
#ifdef OSX
173178
[GLContextClass clearCurrentContext];
174179
[g_ctx clearDrawable];
175-
RELEASE(g_ctx);
176180
if (g_hw_ctx)
177181
[g_hw_ctx clearDrawable];
178-
RELEASE(g_hw_ctx);
179182
[GLContextClass clearCurrentContext];
180183
#else
181184
/* Clean up GLKView's framebuffer resources while context is still valid.
@@ -185,8 +188,18 @@ static void cocoa_gl_gfx_ctx_destroy(void *data)
185188
[glk_view deleteDrawable];
186189
[EAGLContext setCurrentContext:nil];
187190
#endif
188-
g_hw_ctx = nil;
189-
g_ctx = nil;
191+
/* RELEASE -releases + nils under MRR, just nils under ARC (see
192+
* cocoa_common.h). Doing this unconditionally matches the OSX
193+
* path's previous behaviour and fixes a latent iOS-MRR leak where
194+
* the +1 from [[EAGLContext alloc] initWithAPI:...] in set_video_mode
195+
* was dropped with a raw 'g_ctx = nil'. */
196+
RELEASE(g_ctx);
197+
RELEASE(g_hw_ctx);
198+
#if defined(HAVE_COCOATOUCH)
199+
/* glk_view was created with +new (+1) in glkitview_init. Release
200+
* it on destroy so it does not leak across init/destroy cycles. */
201+
RELEASE(glk_view);
202+
#endif
190203

191204
free(cocoa_ctx);
192205
}
@@ -428,11 +441,21 @@ static bool cocoa_gl_gfx_ctx_set_video_mode(void *data,
428441

429442
if (cocoa_ctx->flags & COCOA_CTX_FLAG_USE_HW_CTX)
430443
{
444+
/* In the normal reinit flow -destroy runs before -set_video_mode
445+
* and both statics are already nil here; guard defensively so an
446+
* MRR build does not leak the previous +1 if that invariant ever
447+
* breaks (e.g. a future caller that re-inits without tearing
448+
* down first). Safe when already nil under both ARC and MRR. */
449+
RELEASE(g_ctx);
450+
RELEASE(g_hw_ctx);
431451
g_hw_ctx = [[NSOpenGLContext alloc] initWithFormat:fmt shareContext:nil];
432452
g_ctx = [[NSOpenGLContext alloc] initWithFormat:fmt shareContext:g_hw_ctx];
433453
}
434454
else
455+
{
456+
RELEASE(g_ctx);
435457
g_ctx = [[NSOpenGLContext alloc] initWithFormat:fmt shareContext:nil];
458+
}
436459

437460
RELEASE(fmt);
438461
}
@@ -611,6 +634,13 @@ static bool cocoa_gl_gfx_ctx_set_video_mode(void *data,
611634
{
612635
cocoa_ctx_data_t *cocoa_ctx = (cocoa_ctx_data_t*)data;
613636

637+
/* In the normal reinit flow -destroy runs before -set_video_mode and
638+
* both statics are already nil here; guard defensively so an iOS-MRR
639+
* build does not leak the previous +1 if that invariant ever breaks
640+
* (e.g. a future caller that re-inits without tearing down first). */
641+
RELEASE(g_ctx);
642+
RELEASE(g_hw_ctx);
643+
614644
#if defined(HAVE_OPENGLES3)
615645
if (cocoa_ctx->flags & COCOA_CTX_FLAG_USE_HW_CTX)
616646
{

0 commit comments

Comments
 (0)