From 554a4143028f610894fd0266168f76ce978508f8 Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Mon, 28 Apr 2025 13:55:57 +0300 Subject: [PATCH] Fix remaining memory leaks caused by API misuse --- client/CServerHandler.cpp | 5 +++++ client/eventsSDL/InputHandler.cpp | 5 +---- client/media/CVideoHandler.cpp | 5 +++++ client/renderSDL/SDLImage.cpp | 3 ++- client/renderSDL/SDLImageScaler.cpp | 4 ++-- client/renderSDL/ScalableImage.cpp | 3 ++- 6 files changed, 17 insertions(+), 8 deletions(-) diff --git a/client/CServerHandler.cpp b/client/CServerHandler.cpp index 5573c0b8e..afe4d462e 100644 --- a/client/CServerHandler.cpp +++ b/client/CServerHandler.cpp @@ -57,6 +57,7 @@ #include "LobbyClientNetPackVisitors.h" #include +#include #include @@ -114,9 +115,13 @@ void CServerHandler::threadRunNetwork() } catch (const TerminationRequestedException &) { + // VCMI can run SDL methods on network thread, leading to usage of thread-local storage by SDL + // Such storage needs to be cleaned up manually for threads that were not created by SDL + SDL_TLSCleanup(); logGlobal->info("Terminating network thread"); return; } + SDL_TLSCleanup(); logGlobal->info("Ending network thread"); } diff --git a/client/eventsSDL/InputHandler.cpp b/client/eventsSDL/InputHandler.cpp index 5b20905e9..4806df355 100644 --- a/client/eventsSDL/InputHandler.cpp +++ b/client/eventsSDL/InputHandler.cpp @@ -410,7 +410,7 @@ void InputHandler::dispatchMainThread(const std::function & functor) SDL_Event event; event.user.type = SDL_USEREVENT; event.user.code = 0; - event.user.data1 = static_cast (heapFunctor.get()); + event.user.data1 = nullptr; event.user.data2 = nullptr; SDL_PushEvent(&event); @@ -426,9 +426,6 @@ void InputHandler::handleUserEvent(const SDL_UserEvent & current) if (!dispatchedTasks.try_pop(task)) throw std::runtime_error("InputHandler::handleUserEvent received without active task!"); - if (current.data1 != task.get()) - throw std::runtime_error("InputHandler::handleUserEvent received unknown pointer!"); - (*task)(); } diff --git a/client/media/CVideoHandler.cpp b/client/media/CVideoHandler.cpp index f8ca00985..e6beec141 100644 --- a/client/media/CVideoHandler.cpp +++ b/client/media/CVideoHandler.cpp @@ -373,6 +373,11 @@ FFMpegStream::~FFMpegStream() avcodec_free_context(&codecContext); avformat_close_input(&formatContext); + + // for some reason, buffer is managed (e.g. reallocated) by FFmpeg + // however, it must still be freed manually by user + if (context->buffer) + av_free(context->buffer); av_free(context); } diff --git a/client/renderSDL/SDLImage.cpp b/client/renderSDL/SDLImage.cpp index bc1554e48..9be02d437 100644 --- a/client/renderSDL/SDLImage.cpp +++ b/client/renderSDL/SDLImage.cpp @@ -448,5 +448,6 @@ void SDLImageShared::savePalette() SDLImageShared::~SDLImageShared() { SDL_FreeSurface(surf); - SDL_FreePalette(originalPalette); + if (originalPalette) + SDL_FreePalette(originalPalette); } diff --git a/client/renderSDL/SDLImageScaler.cpp b/client/renderSDL/SDLImageScaler.cpp index da9484f5e..c52c92004 100644 --- a/client/renderSDL/SDLImageScaler.cpp +++ b/client/renderSDL/SDLImageScaler.cpp @@ -236,8 +236,8 @@ SDLImageScaler::~SDLImageScaler() { ENGINE->dispatchMainThread([surface = intermediate]() { - // potentially SDL bug, execute SDL_FreeSurface in main thread to avoid thread races to its internal state - // may be fixed somewhere between 2.26.5 - 2.30 + // SDL_FreeSurface must be executed in main thread to avoid thread races to its internal state + // will be no longer necessary in SDL 3 SDL_FreeSurface(surface); }); } diff --git a/client/renderSDL/ScalableImage.cpp b/client/renderSDL/ScalableImage.cpp index c1fda7d94..a7a8746f8 100644 --- a/client/renderSDL/ScalableImage.cpp +++ b/client/renderSDL/ScalableImage.cpp @@ -90,7 +90,8 @@ ScalableImageParameters::ScalableImageParameters(const SDL_Palette * originalPal ScalableImageParameters::~ScalableImageParameters() { - SDL_FreePalette(palette); + if (palette) + SDL_FreePalette(palette); } void ScalableImageParameters::preparePalette(const SDL_Palette * originalPalette, EImageBlitMode blitMode)