From cab3defc1826b299f55712beb6d6303186ddd8f0 Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <icculus@icculus.org>
Date: Thu, 18 Apr 2024 10:20:31 -0400
Subject: [PATCH] render: SDL_DestroyWindow hollows out its renderer but
 doesn't free it.

This allows apps to destroy the window and renderer in either order, but
makes sure that the renderer can properly clean up its resources while OpenGL
contexts and libraries are still loaded, etc.

If the window is destroyed first, the renderer is (mostly) destroyed but its
pointer remains valid. Attempts to use the renderer will return an error,
but it can still be explicitly destroyed, at which time the struct is free'd.

If the renderer is destroyed first, everything works as before, and a new
renderer can still be created on the existing window.

Fixes #9540.
---
 src/render/SDL_render.c    | 32 ++++++++++++++++++++++++++------
 src/render/SDL_sysrender.h |  5 +++++
 src/video/SDL_video.c      |  3 ++-
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/render/SDL_render.c b/src/render/SDL_render.c
index 2131da194..4f92ea898 100644
--- a/src/render/SDL_render.c
+++ b/src/render/SDL_render.c
@@ -46,12 +46,19 @@ this should probably be removed at some point in the future.  --ryan. */
 #define SDL_PROP_WINDOW_RENDERER_POINTER "SDL.internal.window.renderer"
 #define SDL_PROP_TEXTURE_PARENT_POINTER "SDL.internal.texture.parent"
 
-#define CHECK_RENDERER_MAGIC(renderer, retval)                  \
+#define CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer, retval)                  \
     if (!(renderer) || (renderer)->magic != &SDL_renderer_magic) { \
         SDL_InvalidParamError("renderer");                      \
         return retval;                                          \
     }
 
+#define CHECK_RENDERER_MAGIC(renderer, retval)                  \
+    CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer, retval); \
+    if ((renderer)->destroyed) { \
+        SDL_SetError("Renderer's window has been destroyed, can't use further"); \
+        return retval;                                          \
+    }
+
 #define CHECK_TEXTURE_MAGIC(texture, retval)                    \
     if (!(texture) || (texture)->magic != &SDL_texture_magic) { \
         SDL_InvalidParamError("texture");                       \
@@ -4517,9 +4524,12 @@ static void SDL_DiscardAllCommands(SDL_Renderer *renderer)
     }
 }
 
-void SDL_DestroyRenderer(SDL_Renderer *renderer)
+void SDL_DestroyRendererWithoutFreeing(SDL_Renderer *renderer)
 {
-    CHECK_RENDERER_MAGIC(renderer,);
+    SDL_assert(renderer != NULL);
+    SDL_assert(!renderer->destroyed);
+
+    renderer->destroyed = SDL_TRUE;
 
     SDL_DestroyProperties(renderer->props);
 
@@ -4540,15 +4550,25 @@ void SDL_DestroyRenderer(SDL_Renderer *renderer)
         SDL_ClearProperty(SDL_GetWindowProperties(renderer->window), SDL_PROP_WINDOW_RENDERER_POINTER);
     }
 
-    /* It's no longer magical... */
-    renderer->magic = NULL;
-
     /* Free the target mutex */
     SDL_DestroyMutex(renderer->target_mutex);
     renderer->target_mutex = NULL;
 
     /* Clean up renderer-specific resources */
     renderer->DestroyRenderer(renderer);
+}
+
+void SDL_DestroyRenderer(SDL_Renderer *renderer)
+{
+    CHECK_RENDERER_MAGIC_BUT_NOT_DESTROYED_FLAG(renderer,);
+
+    // if we've already destroyed the renderer through SDL_DestroyWindow, we just need
+    // to free the renderer pointer. This lets apps destroy the window and renderer
+    // in either order.
+    if (!renderer->destroyed) {
+        SDL_DestroyRendererWithoutFreeing(renderer);
+        renderer->magic = NULL;     // It's no longer magical...
+    }
 
     SDL_free(renderer);
 }
diff --git a/src/render/SDL_sysrender.h b/src/render/SDL_sysrender.h
index 3ab7a269a..b26c5752b 100644
--- a/src/render/SDL_sysrender.h
+++ b/src/render/SDL_sysrender.h
@@ -289,6 +289,8 @@ struct SDL_Renderer
 
     SDL_PropertiesID props;
 
+    SDL_bool destroyed;   // already destroyed by SDL_DestroyWindow; just free this struct in SDL_DestroyRenderer.
+
     void *driverdata;
 };
 
@@ -335,6 +337,9 @@ extern SDL_BlendOperation SDL_GetBlendModeAlphaOperation(SDL_BlendMode blendMode
    the next call, because it might be in an array that gets realloc()'d. */
 extern void *SDL_AllocateRenderVertices(SDL_Renderer *renderer, const size_t numbytes, const size_t alignment, size_t *offset);
 
+// Let the video subsystem destroy a renderer without making its pointer invalid.
+extern void SDL_DestroyRendererWithoutFreeing(SDL_Renderer *renderer);
+
 /* Ends C function definitions when using C++ */
 #ifdef __cplusplus
 }
diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c
index 962057f86..4d05869bf 100644
--- a/src/video/SDL_video.c
+++ b/src/video/SDL_video.c
@@ -34,6 +34,7 @@
 #include "../SDL_properties_c.h"
 #include "../timer/SDL_timer_c.h"
 #include "../camera/SDL_camera_c.h"
+#include "../render/SDL_sysrender.h"
 
 #ifdef SDL_VIDEO_OPENGL
 #include <SDL3/SDL_opengl.h>
@@ -3649,7 +3650,7 @@ void SDL_DestroyWindow(SDL_Window *window)
 
     SDL_Renderer *renderer = SDL_GetRenderer(window);
     if (renderer) {
-        SDL_DestroyRenderer(renderer);
+        SDL_DestroyRendererWithoutFreeing(renderer);
     }
 
     SDL_DestroyProperties(window->props);