mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 08:10:30 -10:00
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:
1 parent
1d2dc27294
commit
cff40c8cde
1 file changed
+24
-5
@@ -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];
|
||||
|
||||
Reference in new issue
Block a user