Skip to content

Commit e9279a8

Browse files
committed
OSX/MACOS: Plug MRR leaks in RetroArch_OSX ivar lifetimes
Follow-up to 9743d1e. Audit of the other unretained id ivars in ui_cocoa.m surfaced three smaller lifetime bugs. Unlike the _sleepActivity crash they do not dangle - the referenced objects are kept alive by a second owner (NSWindow.contentView, the _listener delegate slot, g_instance) - so they manifest as leaks rather than EXC_BAD_ACCESS, but they are strictly incorrect under MRR and worth cleaning up while the area is warm. - _renderView: normalise the three -setViewType: paths to a +1 ownership invariant. [MetalView new] already hands us +1, so the Metal path is unchanged. The VULKAN and OPENGL paths get an explicit RARCH_RETAIN around [CocoaView get], which returns an unretained pointer to a singleton. The teardown block then gets a matching RARCH_RELEASE(_renderView) before nilling, so a view-type switch away from Metal no longer leaks the MetalView and switches between the singleton-based types no longer over-release. - CAMetalLayer: the VULKAN path was +alloc/+init-ing a layer and handing it to _renderView.layer (a strong reference, so the view took its own retain) without balancing our +1. Autorelease the local so the layer is freed once the view drops it. - _listener (HAVE_COCOA_METAL only): created with +new (+1) in -applicationDidFinishLaunching: and wired up as the window's delegate and nextResponder - both of which are non-retaining relationships, so the ivar was the sole owner. Release it in -dealloc (MRR branch) so the listener is not leaked at shutdown. No behavioural change under ARC, where strong ivars already track these retains automatically and the RARCH_RETAIN / RARCH_RELEASE macros expand to no-ops. The pre-10.9 SDK compatibility guard stays scoped to the App Nap code path from 9743d1e - none of the APIs touched here are version-sensitive.
1 parent 9743d1e commit e9279a8

1 file changed

Lines changed: 31 additions & 2 deletions

File tree

ui/drivers/ui_cocoa.m

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,17 @@ - (void)dealloc
630630
[pi endActivity:token];
631631
RARCH_RELEASE(token);
632632
}
633+
#endif
634+
/* _renderView is kept at +1 by setViewType:; release the balance
635+
* here so the last view-type's render view does not leak at
636+
* shutdown. Safe when nil. */
637+
RARCH_RELEASE(_renderView);
638+
#ifdef HAVE_COCOA_METAL
639+
/* _listener was created with +new (+1) in applicationDidFinishLaunching:
640+
* and installed as the window's delegate / nextResponder, which are
641+
* unretained relationships. Release our own retain so the listener
642+
* does not leak at shutdown. */
643+
RARCH_RELEASE(_listener);
633644
#endif
634645
RARCH_RELEASE(_window);
635646
RARCH_SUPER_DEALLOC();
@@ -719,32 +730,50 @@ - (void)setViewType:(apple_view_type_t)vt
719730
_renderView.layer = nil;
720731
[_renderView removeFromSuperview];
721732
self.window.contentView = nil;
733+
/* _renderView holds a +1 retain regardless of which path created
734+
* it below (see the RARCH_RETAIN on the [CocoaView get] paths,
735+
* and the inherent +1 from +new on the Metal path). Release it
736+
* here so the ownership invariant is balanced before we nil the
737+
* ivar. Under ARC this is a no-op and the implicit __strong
738+
* ivar handles the release when _renderView is assigned nil. */
739+
RARCH_RELEASE(_renderView);
722740
_renderView = nil;
723741
}
724742

725743
switch (vt)
726744
{
727745
case APPLE_VIEW_TYPE_VULKAN:
728746
{
729-
_renderView = [CocoaView get];
747+
/* [CocoaView get] returns an unretained pointer to a
748+
* singleton. Retain explicitly so _renderView's +1
749+
* ownership invariant matches the Metal path below. */
750+
_renderView = RARCH_RETAIN([CocoaView get]);
730751
CAMetalLayer *metal_layer = [[CAMetalLayer alloc] init];
731752
metal_layer.device = MTLCreateSystemDefaultDevice();
732753
metal_layer.framebufferOnly = YES;
733754
metal_layer.contentsScale = [[NSScreen mainScreen] backingScaleFactor];
755+
/* CALayer.layer is a strong reference, so the view takes
756+
* its own retain. Autorelease our +1 from +alloc/+init
757+
* so we don't leak the layer on every view-type switch. */
734758
_renderView.layer = metal_layer;
759+
RARCH_AUTORELEASE(metal_layer);
735760
[_renderView setWantsLayer:YES];
736761
}
737762
break;
738763
case APPLE_VIEW_TYPE_METAL:
739764
{
765+
/* +new returns a +1 object; that retain transfers into
766+
* _renderView and satisfies the ivar's ownership
767+
* invariant directly. No extra RARCH_RETAIN needed. */
740768
MetalView *v = [MetalView new];
741769
v.paused = YES;
742770
v.enableSetNeedsDisplay = NO;
743771
_renderView = v;
744772
}
745773
break;
746774
case APPLE_VIEW_TYPE_OPENGL:
747-
_renderView = [CocoaView get];
775+
/* Same singleton-retain story as the VULKAN case. */
776+
_renderView = RARCH_RETAIN([CocoaView get]);
748777
break;
749778
case APPLE_VIEW_TYPE_NONE:
750779
default:

0 commit comments

Comments
 (0)