ui_cocoatouch: the app icon texture cache was autoreleased into a static

Looking at whether this dispatch_once was needed turned up something
worse than the once.

The cache is +dictionaryWithCapacity:, which hands back an autoreleased
object, assigned to a static and used across calls. This file is built
MRC by the Makefile - it is not in the -fobjc-arc list at Makefile:275
- so the pool drains at the end of the run loop pass that created it,
and every later call messages freed memory. Under Xcode, where
griffin_objc.m is compiled ARC, the strong static retains it and the
identical code is correct, which is how this has survived: the build
most people use is the one where it works, and the menu that reaches
it - materialui's app icon list - is not a screen most people open.
-initWithCapacity: is +1 owned and right under both, and the object is
kept for the process lifetime deliberately, as the dock indicator in
dispserv_apple.m is.

The dispatch_once around that create was misleading rather than
wrong. It reads as a thread-safety guarantee the rest of the function
does not make: the next lines mutate the dictionary with no
synchronisation at all, so were this ever reached from two threads the
once would be the one part that was safe. The sole caller is
materialui's icon draw, on the menu's frame, so a plain lazy create
says what is true.

Left alone, the other three in this sweep. WebServer's +sharedInstance
is a public singleton accessor whose contract is that it works from
any thread, and it constructs a real object - that is what
dispatch_once is for, and swapping it for a flag would trade a correct
idiom for a proof obligation the next caller has to know about.
get_app_icons() and get_ios_version() likewise build something once,
from main-thread paths, at no cost worth removing. Unlike the metal
and cocoa_common cases, none of these three is guarding the assignment
of constants into a global.

Not compiled here - no Apple toolchain, UIKit cannot be stubbed.
Linux still builds.
This commit is contained in:
LibretroAdmin authored and libretroadmin committed 2026-09-04 16:27:00 +02:00
1 parent 1d2dc27294
commit cff40c8cde
1 file changed
+24 -5
+24 -5
View File
@@ -156,15 +156,34 @@ static void ui_companion_cocoatouch_set_app_icon(const char *iconName)
[[UIApplication sharedApplication] setAlternateIconName:str completionHandler:nil];
}
/* Main thread only: the sole caller is materialui's icon draw, which
* runs from the menu's frame.
*
* The cache used to be created under dispatch_once, which read as a
* thread-safety guarantee the rest of the function does not make - the
* very next lines mutate the dictionary with no synchronisation at
* all, so were this ever reached from two threads the once would be
* the one part that was safe. A plain lazy create says what is true:
* one thread, so neither the create nor the mutation needs guarding.
*
* +dictionaryWithCapacity: hands back an autoreleased object, which is
* wrong for something a static holds across calls: this file is built
* MRC by the Makefile - it is not in the -fobjc-arc list at Makefile:275
* - so the pool drains at the end of the run loop pass that created it
* and every later call messages freed memory. Under Xcode, where
* griffin_objc.m is ARC, the strong static retains it and the same
* code is fine, which is why this has sat here. -initWithCapacity: is
* +1 owned and correct in both: the object is kept for the process
* lifetime deliberately, as the dock indicator in dispserv_apple.m is. */
static uintptr_t ui_companion_cocoatouch_get_app_icon_texture(const char *icon)
{
static NSMutableDictionary<NSString *, NSNumber *> *textures = nil;
static dispatch_once_t once;
dispatch_once(&once, ^{
textures = [NSMutableDictionary dictionaryWithCapacity:6];
});
NSString *iconName;
NSString *iconName = [NSString stringWithUTF8String:icon];
if (!textures)
textures = [[NSMutableDictionary alloc] initWithCapacity:6];
iconName = [NSString stringWithUTF8String:icon];
if (!textures[iconName])
{
UIImage *img = [UIImage imageNamed:iconName];