Skip to content

Commit 93cdcb8

Browse files
committed
gfx/dispserv_apple: plug two MRC leaks in macOS progress/resolution paths
This file compiles under MRC (the top-level Makefile only flips -fobjc-arc on for gfx/drivers/metal.o and input/drivers_joypad/ mfi_joypad.o). Two leaks that MRC exposes but ARC would have auto-cleaned: === Leak 1: NSImageView 'iv' inside dispatch_once === apple_display_server_set_window_progress builds the dock-tile progress indicator once per process: dispatch_once(&once, ^{ NSDockTile *dockTile = [NSApp dockTile]; NSImageView *iv = [[NSImageView alloc] init]; /* +1 */ [iv setImage:...]; [dockTile setContentView:iv]; /* +1 parent-owned */ indicator = [[NSProgressIndicator alloc] initWithFrame:...]; [indicator setIndeterminate:NO]; ... [iv addSubview:indicator]; /* +1 parent-owned */ }); At end of the block, 'iv' the local goes out of scope with a retain count around +3. The two parent-view retains are legitimately owned; the +1 from alloc+init is ours to release and was getting leaked for the app's lifetime. Single-shot leak (dispatch_once guards it to run exactly once) and only tens of bytes, but still a genuine leak that a leaks(1) run would flag. 'indicator' stays intentionally +1 - it's a file-scope static referenced from outside the block for subsequent setDoubleValue / setHidden calls. The block-local strong hand on it is the reason it survives across dispatch_once invocations. Fix: '[iv release]' at end of block. === Leak 2: displayModes + currentMode on OOM in get_resolution_list === apple_display_server_get_resolution_list acquires two CGDisplay-family CF refs at the top of its macOS branch: CGDisplayModeRef currentMode = CGDisplayCopyDisplayMode(mainDisplayID); CFArrayRef displayModes = CGDisplayCopyAllDisplayModes(mainDisplayID, NULL); Both are +1-owned under the Core Foundation 'Copy' naming rule. The success path correctly CFRelease's both before returning (lines 339-340 pre-patch). The OOM path for the output-array calloc at line 330 returned NULL without touching either: if (!(conf = (struct video_display_config*)calloc(*len, sizeof(...)))) return NULL; /* leaks displayModes + currentMode */ Fix: CFRelease both on the OOM exit. Small objects but a leak of system-owned Core Graphics state that persists across repeated resolution-menu openings (the function is called every time the user opens Settings > Video > Resolution on macOS). Thread-safety: unchanged. Both functions run on the main AppKit thread. Reachability: the progress indicator runs once per RetroArch process on macOS when a task first reports progress via the dock; the resolution-list function runs from menu open whenever the user navigates to the display-settings page. Neither is a hot path, but both have been accumulating the leaks described above on every macOS-MRC build since they were introduced.
1 parent c09794c commit 93cdcb8

1 file changed

Lines changed: 26 additions & 0 deletions

File tree

gfx/display_servers/dispserv_apple.m

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,20 @@ static bool apple_display_server_set_window_progress(void *data, int progress, b
8383

8484
// Create a custom view for the dock tile
8585
[iv addSubview:indicator];
86+
87+
/* Under MRC (this file compiles under -fobjc-no-arc per the
88+
* top-level Makefile), 'iv' is a local +1 from alloc+init.
89+
* setContentView: and addSubview: each retained it for their
90+
* own ownership, so at this point its true retain count is
91+
* roughly +3. The two parent-view retains are legitimately
92+
* owned; the +1 from alloc+init is ours to release before
93+
* the local goes out of scope, or it leaks for the app's
94+
* lifetime. 'indicator' is intentionally leaked here - it's
95+
* a file-scope static initialised inside dispatch_once, and
96+
* holding that +1 across the app lifetime is how we keep it
97+
* referenced for the subsequent setDoubleValue / setHidden
98+
* calls outside the block. */
99+
[iv release];
86100
});
87101
if (finished)
88102
[indicator setDoubleValue:(double)-1];
@@ -328,7 +342,19 @@ static bool apple_display_server_set_resolution(void *data,
328342
/* Set length and allocate config array for macOS */
329343
*len = (unsigned)[configArray count];
330344
if (!(conf = (struct video_display_config*)calloc(*len, sizeof(struct video_display_config))))
345+
{
346+
/* displayModes and currentMode are CFRetain'd +1 by their
347+
* respective CGDisplayCopy* calls above (~line 250 / 266)
348+
* and must be CFRelease'd on every exit path. The success
349+
* return below does this at lines 339-340; the pre-patch
350+
* OOM path bypassed both and leaked them until the process
351+
* died. Each is a small CF object (handful of bytes) but
352+
* a leak of system-owned Core Graphics state is still
353+
* worth plugging. */
354+
CFRelease(displayModes);
355+
CFRelease(currentMode);
331356
return NULL;
357+
}
332358

333359
for (j = 0; j < *len; j++)
334360
{

0 commit comments

Comments
 (0)