mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 08:10:30 -10:00
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.
This commit is contained in:
1 parent
c09794c698
commit
93cdcb844d
1 file changed
+26
@@ -83,6 +83,20 @@ static bool apple_display_server_set_window_progress(void *data, int progress, b
|
||||
|
||||
// Create a custom view for the dock tile
|
||||
[iv addSubview:indicator];
|
||||
|
||||
/* Under MRC (this file compiles under -fobjc-no-arc per the
|
||||
* top-level Makefile), 'iv' is a local +1 from alloc+init.
|
||||
* setContentView: and addSubview: each retained it for their
|
||||
* own ownership, so at this point its true retain count is
|
||||
* roughly +3. The two parent-view retains are legitimately
|
||||
* owned; the +1 from alloc+init is ours to release before
|
||||
* the local goes out of scope, or it leaks for the app's
|
||||
* lifetime. 'indicator' is intentionally leaked here - it's
|
||||
* a file-scope static initialised inside dispatch_once, and
|
||||
* holding that +1 across the app lifetime is how we keep it
|
||||
* referenced for the subsequent setDoubleValue / setHidden
|
||||
* calls outside the block. */
|
||||
[iv release];
|
||||
});
|
||||
if (finished)
|
||||
[indicator setDoubleValue:(double)-1];
|
||||
@@ -328,7 +342,19 @@ static void *apple_display_server_get_resolution_list(
|
||||
/* Set length and allocate config array for macOS */
|
||||
*len = (unsigned)[configArray count];
|
||||
if (!(conf = (struct video_display_config*)calloc(*len, sizeof(struct video_display_config))))
|
||||
{
|
||||
/* displayModes and currentMode are CFRetain'd +1 by their
|
||||
* respective CGDisplayCopy* calls above (~line 250 / 266)
|
||||
* and must be CFRelease'd on every exit path. The success
|
||||
* return below does this at lines 339-340; the pre-patch
|
||||
* OOM path bypassed both and leaked them until the process
|
||||
* died. Each is a small CF object (handful of bytes) but
|
||||
* a leak of system-owned Core Graphics state is still
|
||||
* worth plugging. */
|
||||
CFRelease(displayModes);
|
||||
CFRelease(currentMode);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
for (j = 0; j < *len; j++)
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user