Skip to content

Commit 9743d1e

Browse files
committed
OSX/MACOS: Fix crash when toggling Suspend Screensaver
-[RetroArch_OSX setDisableDisplaySleep:] stored the NSActivity token returned by -beginActivityWithOptions:reason: in a raw-id ivar with no retain. The token is autoreleased and owned by the system, so under MRR (used by RetroArch_PPC.xcodeproj and the qb/make build - ARC is not universal in this file) it was deallocated as soon as the surrounding autorelease pool drained, leaving _sleepActivity as a dangling pointer. Toggling the setting back off then sent -endActivity: to freed memory, which crashed in objc_opt_isKindOfClass at offset 0x1c while checking -isKindOfClass:_NSActivityAssertion. Fixes (macOS 12.3, x86_64, 1.22.2): 0 libobjc objc_opt_isKindOfClass + 27 1 Foundation -[NSProcessInfo endActivity:] + 29 2 RetroArch -[RetroArch_OSX setDisableDisplaySleep:] + 125 3 RetroArch general_write_handler + 1646 4 RetroArch setting_bool_action_ok_default + 34 ... - Retain the token (RARCH_RETAIN, no-op under ARC, -retain under MRR) so it survives across the begin/end pair. - Capture + nil-out the ivar before calling -endActivity: to make a re-entrant call from inside AppKit's menu write path unable to observe a stale pointer and double-end. - Release any outstanding activity in -dealloc (MRR branch) so quitting while the screensaver is suspended does not leak the system-wide display-sleep assertion. - Guard the whole method with MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 so 10.5-era SDKs (Xcode 3.1 / GCC 4.0, which predate both App Nap and ARC) still compile, and with -respondsToSelector: so a binary built on a newer SDK but deployed to 10.5-10.8 no-ops instead of crashing on unrecognized selector. Also adds RARCH_RETAIN to cocoa_defines.h next to the existing RARCH_RELEASE / RARCH_AUTORELEASE / RARCH_SUPER_DEALLOC family.
1 parent b0f6690 commit 9743d1e

2 files changed

Lines changed: 54 additions & 2 deletions

File tree

libretro-common/include/defines/cocoa_defines.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,12 @@
9393
#endif
9494

9595
#if __has_feature(objc_arc)
96+
#define RARCH_RETAIN(x) (x)
9697
#define RARCH_RELEASE(x) ((void)0)
9798
#define RARCH_AUTORELEASE(x) ((void)0)
9899
#define RARCH_SUPER_DEALLOC() ((void)0)
99100
#else
101+
#define RARCH_RETAIN(x) [(x) retain]
100102
#define RARCH_RELEASE(x) [(x) release]
101103
#define RARCH_AUTORELEASE(x) [(x) autorelease]
102104
#define RARCH_SUPER_DEALLOC() [super dealloc]

ui/drivers/ui_cocoa.m

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,22 @@ @implementation RetroArch_OSX
615615
* _window manually. See cocoa_defines.h for the ARC/MRR macro story. */
616616
- (void)dealloc
617617
{
618+
/* Make sure any outstanding NSProcessInfo activity is ended and
619+
* released. Without this, quitting while the screensaver is
620+
* suspended would leak both the activity assertion (leaving
621+
* display-sleep disabled system-wide until reboot) and the
622+
* retained token itself. */
623+
#if defined(MAC_OS_X_VERSION_MAX_ALLOWED) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
624+
if (_sleepActivity)
625+
{
626+
NSProcessInfo *pi = [NSProcessInfo processInfo];
627+
id token = _sleepActivity;
628+
_sleepActivity = nil;
629+
if ([pi respondsToSelector:@selector(endActivity:)])
630+
[pi endActivity:token];
631+
RARCH_RELEASE(token);
632+
}
633+
#endif
618634
RARCH_RELEASE(_window);
619635
RARCH_SUPER_DEALLOC();
620636
}
@@ -808,20 +824,54 @@ - (void)setCursorVisible:(bool)v
808824

809825
- (bool)setDisableDisplaySleep:(bool)disable
810826
{
827+
/* beginActivityWithOptions:reason: / endActivity: were added in
828+
* macOS 10.9. Guard at compile time so builds against older SDKs
829+
* (notably 10.5 via Xcode 3.1 / GCC 4.0, which predate both App
830+
* Nap and ARC) still compile, and guard at runtime so a binary
831+
* built against a 10.9+ SDK but deployed onto 10.5-10.8 gracefully
832+
* no-ops instead of crashing on an unrecognized selector. */
833+
#if defined(MAC_OS_X_VERSION_MAX_ALLOWED) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
834+
NSProcessInfo *pi = [NSProcessInfo processInfo];
835+
if (![pi respondsToSelector:@selector(beginActivityWithOptions:reason:)])
836+
return NO;
837+
811838
if (disable)
812839
{
813840
if (_sleepActivity == nil)
814-
_sleepActivity = [NSProcessInfo.processInfo beginActivityWithOptions:NSActivityIdleDisplaySleepDisabled reason:@"disable screen saver"];
841+
{
842+
/* The token returned by beginActivityWithOptions:reason: is
843+
* autoreleased and owned by the system. We MUST retain it
844+
* or, under MRR, it is deallocated as soon as the current
845+
* autorelease pool drains - leaving _sleepActivity as a
846+
* dangling pointer. The next call to endActivity: then
847+
* sends isKindOfClass: to freed memory and crashes in
848+
* objc_opt_isKindOfClass. Under ARC a plain `id` ivar is
849+
* implicitly __strong so RARCH_RETAIN is a no-op; under MRR
850+
* it expands to an explicit -retain. */
851+
id token = [pi beginActivityWithOptions:NSActivityIdleDisplaySleepDisabled
852+
reason:@"disable screen saver"];
853+
_sleepActivity = RARCH_RETAIN(token);
854+
}
815855
}
816856
else
817857
{
818858
if (_sleepActivity)
819859
{
820-
[NSProcessInfo.processInfo endActivity:_sleepActivity];
860+
/* Capture and nil-out BEFORE calling endActivity: so that a
861+
* re-entrant call (for example from a menu write handler
862+
* triggered while AppKit is dispatching) cannot observe a
863+
* stale pointer and double-end the same activity. */
864+
id token = _sleepActivity;
821865
_sleepActivity = nil;
866+
[pi endActivity:token];
867+
RARCH_RELEASE(token);
822868
}
823869
}
824870
return YES;
871+
#else
872+
(void)disable;
873+
return NO;
874+
#endif
825875
}
826876
#endif
827877

0 commit comments

Comments
 (0)