From d6212ae8397d09a48099d85279f6652d056a3958 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Mon, 19 May 2025 13:53:33 -0700 Subject: [PATCH] Fixed rare crash trying to interrupt SDL_WaitEvent() Fixes https://github.com/libsdl-org/SDL/issues/12797 (cherry picked from commit 992e4c59bdf03dbec8b1689332ee697623cf6ad2) --- src/events/SDL_events.c | 32 ++++++---------------------- src/video/SDL_sysvideo.h | 3 +-- src/video/SDL_video.c | 4 +--- src/video/cocoa/SDL_cocoavideo.m | 4 ---- src/video/wayland/SDL_waylandvideo.c | 4 ---- src/video/windows/SDL_windowsvideo.c | 4 ---- src/video/x11/SDL_x11video.c | 5 ----- 7 files changed, 9 insertions(+), 47 deletions(-) diff --git a/src/events/SDL_events.c b/src/events/SDL_events.c index eff205f71b..a151740524 100644 --- a/src/events/SDL_events.c +++ b/src/events/SDL_events.c @@ -1077,16 +1077,11 @@ static void SDL_SendWakeupEvent(void) return; } - SDL_LockMutex(_this->wakeup_lock); - { - if (_this->wakeup_window) { - _this->SendWakeupEvent(_this, _this->wakeup_window); - - // No more wakeup events needed until we enter a new wait - _this->wakeup_window = NULL; - } + // We only want to do this once while waiting for an event, so set it to NULL atomically here + SDL_Window *wakeup_window = (SDL_Window *)SDL_SetAtomicPointer(&_this->wakeup_window, NULL); + if (wakeup_window) { + _this->SendWakeupEvent(_this, wakeup_window); } - SDL_UnlockMutex(_this->wakeup_lock); #endif } @@ -1524,18 +1519,7 @@ static int SDL_WaitEventTimeout_Device(SDL_VideoDevice *_this, SDL_Window *wakeu */ SDL_PumpEventsInternal(true); - SDL_LockMutex(_this->wakeup_lock); - { - status = SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_EVENT_FIRST, SDL_EVENT_LAST); - // If status == 0 we are going to block so wakeup will be needed. - if (status == 0) { - _this->wakeup_window = wakeup_window; - } else { - _this->wakeup_window = NULL; - } - } - SDL_UnlockMutex(_this->wakeup_lock); - + status = SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_EVENT_FIRST, SDL_EVENT_LAST); if (status < 0) { // Got an error: return break; @@ -1548,8 +1532,6 @@ static int SDL_WaitEventTimeout_Device(SDL_VideoDevice *_this, SDL_Window *wakeu if (timeoutNS > 0) { Sint64 elapsed = SDL_GetTicksNS() - start; if (elapsed >= timeoutNS) { - // Set wakeup_window to NULL without holding the lock. - _this->wakeup_window = NULL; return 0; } loop_timeoutNS = (timeoutNS - elapsed); @@ -1562,9 +1544,9 @@ static int SDL_WaitEventTimeout_Device(SDL_VideoDevice *_this, SDL_Window *wakeu loop_timeoutNS = poll_intervalNS; } } + SDL_SetAtomicPointer(&_this->wakeup_window, wakeup_window); status = _this->WaitEventTimeout(_this, loop_timeoutNS); - // Set wakeup_window to NULL without holding the lock. - _this->wakeup_window = NULL; + SDL_SetAtomicPointer(&_this->wakeup_window, NULL); if (status == 0 && poll_intervalNS != SDL_MAX_SINT64 && loop_timeoutNS == poll_intervalNS) { // We may have woken up to poll. Try again continue; diff --git a/src/video/SDL_sysvideo.h b/src/video/SDL_sysvideo.h index 6da8bd2e18..ad5bed288b 100644 --- a/src/video/SDL_sysvideo.h +++ b/src/video/SDL_sysvideo.h @@ -396,8 +396,7 @@ struct SDL_VideoDevice bool checked_texture_framebuffer; bool is_dummy; bool suspend_screensaver; - SDL_Window *wakeup_window; - SDL_Mutex *wakeup_lock; // Initialized only if WaitEventTimeout/SendWakeupEvent are supported + void *wakeup_window; int num_displays; SDL_VideoDisplay **displays; SDL_Rect desktop_bounds; diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c index abeb2cb6b5..8932d1e1a9 100644 --- a/src/video/SDL_video.c +++ b/src/video/SDL_video.c @@ -4279,9 +4279,7 @@ void SDL_DestroyWindow(SDL_Window *window) _this->current_glwin = NULL; } - if (_this->wakeup_window == window) { - _this->wakeup_window = NULL; - } + SDL_CompareAndSwapAtomicPointer(&_this->wakeup_window, window, NULL); // Now invalidate magic SDL_SetObjectValid(window, SDL_OBJECT_TYPE_WINDOW, false); diff --git a/src/video/cocoa/SDL_cocoavideo.m b/src/video/cocoa/SDL_cocoavideo.m index 81baf7825f..aed193df44 100644 --- a/src/video/cocoa/SDL_cocoavideo.m +++ b/src/video/cocoa/SDL_cocoavideo.m @@ -49,9 +49,6 @@ static void Cocoa_VideoQuit(SDL_VideoDevice *_this); static void Cocoa_DeleteDevice(SDL_VideoDevice *device) { @autoreleasepool { - if (device->wakeup_lock) { - SDL_DestroyMutex(device->wakeup_lock); - } CFBridgingRelease(device->internal); SDL_free(device); } @@ -81,7 +78,6 @@ static SDL_VideoDevice *Cocoa_CreateDevice(void) return NULL; } device->internal = (SDL_VideoData *)CFBridgingRetain(data); - device->wakeup_lock = SDL_CreateMutex(); device->system_theme = Cocoa_GetSystemTheme(); // Set the function pointers diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c index 43019535f5..47971e6a30 100644 --- a/src/video/wayland/SDL_waylandvideo.c +++ b/src/video/wayland/SDL_waylandvideo.c @@ -446,9 +446,6 @@ static void Wayland_DeleteDevice(SDL_VideoDevice *device) WAYLAND_wl_display_disconnect(data->display); SDL_ClearProperty(SDL_GetGlobalProperties(), SDL_PROP_GLOBAL_VIDEO_WAYLAND_WL_DISPLAY_POINTER); } - if (device->wakeup_lock) { - SDL_DestroyMutex(device->wakeup_lock); - } SDL_free(data); SDL_free(device); SDL_WAYLAND_UnloadSymbols(); @@ -589,7 +586,6 @@ static SDL_VideoDevice *Wayland_CreateDevice(bool require_preferred_protocols) } device->internal = data; - device->wakeup_lock = SDL_CreateMutex(); // Set the function pointers device->VideoInit = Wayland_VideoInit; diff --git a/src/video/windows/SDL_windowsvideo.c b/src/video/windows/SDL_windowsvideo.c index 6103d52334..841a9ecba5 100644 --- a/src/video/windows/SDL_windowsvideo.c +++ b/src/video/windows/SDL_windowsvideo.c @@ -120,9 +120,6 @@ static void WIN_DeleteDevice(SDL_VideoDevice *device) SDL_UnloadObject(data->dxgiDLL); } #endif - if (device->wakeup_lock) { - SDL_DestroyMutex(device->wakeup_lock); - } SDL_free(device->internal->rawinput); SDL_free(device->internal); SDL_free(device); @@ -148,7 +145,6 @@ static SDL_VideoDevice *WIN_CreateDevice(void) return NULL; } device->internal = data; - device->wakeup_lock = SDL_CreateMutex(); device->system_theme = WIN_GetSystemTheme(); #if !defined(SDL_PLATFORM_XBOXONE) && !defined(SDL_PLATFORM_XBOXSERIES) diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c index c188f91f43..37bef7bf8b 100644 --- a/src/video/x11/SDL_x11video.c +++ b/src/video/x11/SDL_x11video.c @@ -63,9 +63,6 @@ static void X11_DeleteDevice(SDL_VideoDevice *device) X11_XCloseDisplay(data->request_display); } SDL_free(data->windowlist); - if (device->wakeup_lock) { - SDL_DestroyMutex(device->wakeup_lock); - } SDL_free(device->internal); SDL_free(device); @@ -146,8 +143,6 @@ static SDL_VideoDevice *X11_CreateDevice(void) return NULL; } - device->wakeup_lock = SDL_CreateMutex(); - #ifdef X11_DEBUG X11_XSynchronize(data->display, True); #endif