From bc63531e0062324166ccee0e2bada40075118923 Mon Sep 17 00:00:00 2001 From: RSDuck Date: Thu, 11 Mar 2021 16:54:27 +0100 Subject: [PATCH] avoid leaking threads in NDSCart_SRAMManager also atomics --- src/GPU3D_Soft.cpp | 10 +- src/GPU3D_Soft.h | 5 +- src/NDSCart_SRAMManager.cpp | 275 +++++++++++++++++++----------------- 3 files changed, 150 insertions(+), 140 deletions(-) diff --git a/src/GPU3D_Soft.cpp b/src/GPU3D_Soft.cpp index 22f7f01c..dd4a3040 100644 --- a/src/GPU3D_Soft.cpp +++ b/src/GPU3D_Soft.cpp @@ -33,7 +33,7 @@ void RenderThreadFunc(); void SoftRenderer::StopRenderThread() { - if (RenderThreadRunning) + if (RenderThreadRunning.load(std::memory_order_relaxed)) { RenderThreadRunning = false; Platform::Semaphore_Post(Sema_RenderStart); @@ -46,7 +46,7 @@ void SoftRenderer::SetupRenderThread() { if (Threaded) { - if (!RenderThreadRunning) + if (!RenderThreadRunning.load(std::memory_order_relaxed)) { RenderThreadRunning = true; RenderThread = Platform::Thread_Create(std::bind(&SoftRenderer::RenderThreadFunc, this)); @@ -1646,7 +1646,7 @@ void SoftRenderer::RenderPolygons(bool threaded, Polygon** polygons, int npolys) void SoftRenderer::VCount144() { - if (RenderThreadRunning) + if (RenderThreadRunning.load(std::memory_order_relaxed)) Platform::Semaphore_Wait(Sema_RenderDone); } @@ -1660,7 +1660,7 @@ void SoftRenderer::RenderFrame() FrameIdentical = !(textureChanged || texPalChanged) && RenderFrameIdentical; - if (RenderThreadRunning) + if (RenderThreadRunning.load(std::memory_order_relaxed)) { Platform::Semaphore_Post(Sema_RenderStart); } @@ -1701,7 +1701,7 @@ void SoftRenderer::RenderThreadFunc() u32* SoftRenderer::GetLine(int line) { - if (RenderThreadRunning) + if (RenderThreadRunning.load(std::memory_order_relaxed)) { if (line < 192) Platform::Semaphore_Wait(Sema_ScanlineCount); diff --git a/src/GPU3D_Soft.h b/src/GPU3D_Soft.h index ee1977d3..9f6f9931 100644 --- a/src/GPU3D_Soft.h +++ b/src/GPU3D_Soft.h @@ -21,6 +21,7 @@ #include "GPU3D.h" #include "Platform.h" #include +#include namespace GPU3D { @@ -506,8 +507,8 @@ private: bool Threaded; Platform::Thread* RenderThread; - bool RenderThreadRunning; - bool RenderThreadRendering; + std::atomic_bool RenderThreadRunning; + std::atomic_bool RenderThreadRendering; Platform::Semaphore* Sema_RenderStart; Platform::Semaphore* Sema_RenderDone; Platform::Semaphore* Sema_ScanlineCount; diff --git a/src/NDSCart_SRAMManager.cpp b/src/NDSCart_SRAMManager.cpp index c93db2a3..49c5357a 100644 --- a/src/NDSCart_SRAMManager.cpp +++ b/src/NDSCart_SRAMManager.cpp @@ -20,156 +20,165 @@ #include #include #include +#include #include "NDSCart_SRAMManager.h" #include "Platform.h" namespace NDSCart_SRAMManager { - Platform::Thread* FlushThread; - bool FlushThreadRunning; - Platform::Mutex* SecondaryBufferLock; - char Path[1024]; +Platform::Thread* FlushThread; +std::atomic_bool FlushThreadRunning; +Platform::Mutex* SecondaryBufferLock; - u8* Buffer; - u32 Length; +char Path[1024]; - u8* SecondaryBuffer; - u32 SecondaryBufferLength; - - time_t TimeAtLastFlushRequest; +u8* Buffer; +u32 Length; - // We keep versions in case the user closes the application before - // a flush cycle is finished. - u32 PreviousFlushVersion; - u32 FlushVersion; +u8* SecondaryBuffer; +u32 SecondaryBufferLength; - void FlushThreadFunc(); +time_t TimeAtLastFlushRequest; - bool Init() +// We keep versions in case the user closes the application before +// a flush cycle is finished. +u32 PreviousFlushVersion; +u32 FlushVersion; + +void FlushThreadFunc(); + +bool Init() +{ + SecondaryBufferLock = Platform::Mutex_Create(); + + return true; +} + +void DeInit() +{ + if (FlushThreadRunning) { - SecondaryBufferLock = Platform::Mutex_Create(); - - return true; - } - - void DeInit() - { - if (FlushThreadRunning) - { - FlushThreadRunning = false; - Platform::Thread_Wait(FlushThread); - Platform::Thread_Free(FlushThread); - FlushSecondaryBuffer(); - } - - if (SecondaryBuffer) delete SecondaryBuffer; - SecondaryBuffer = NULL; - - Platform::Mutex_Free(SecondaryBufferLock); - } - - void Setup(const char* path, u8* buffer, u32 length) - { - // Flush SRAM in case there is unflushed data from previous state. + FlushThreadRunning = false; + Platform::Thread_Wait(FlushThread); + Platform::Thread_Free(FlushThread); FlushSecondaryBuffer(); - - Platform::Mutex_Lock(SecondaryBufferLock); - - strncpy(Path, path, 1023); - Path[1023] = '\0'; - - Buffer = buffer; - Length = length; - - if(SecondaryBuffer) delete SecondaryBuffer; // Delete secondary buffer, there might be previous state. - - SecondaryBuffer = new u8[length]; - SecondaryBufferLength = length; - - FlushVersion = 0; - PreviousFlushVersion = 0; - TimeAtLastFlushRequest = 0; - - Platform::Mutex_Unlock(SecondaryBufferLock); - - if (path[0] != '\0') - { - FlushThread = Platform::Thread_Create(FlushThreadFunc); - FlushThreadRunning = true; - } } - void RequestFlush() + if (SecondaryBuffer) delete[] SecondaryBuffer; + SecondaryBuffer = NULL; + + Platform::Mutex_Free(SecondaryBufferLock); +} + +void Setup(const char* path, u8* buffer, u32 length) +{ + // Flush SRAM in case there is unflushed data from previous state. + FlushSecondaryBuffer(); + + Platform::Mutex_Lock(SecondaryBufferLock); + + strncpy(Path, path, 1023); + Path[1023] = '\0'; + + Buffer = buffer; + Length = length; + + if(SecondaryBuffer) delete[] SecondaryBuffer; // Delete secondary buffer, there might be previous state. + + SecondaryBuffer = new u8[length]; + SecondaryBufferLength = length; + + FlushVersion = 0; + PreviousFlushVersion = 0; + TimeAtLastFlushRequest = 0; + + Platform::Mutex_Unlock(SecondaryBufferLock); + + if (path[0] != '\0' && !FlushThreadRunning) { - Platform::Mutex_Lock(SecondaryBufferLock); - printf("NDS SRAM: Flush requested\n"); - memcpy(SecondaryBuffer, Buffer, Length); - FlushVersion++; - TimeAtLastFlushRequest = time(NULL); - Platform::Mutex_Unlock(SecondaryBufferLock); + FlushThread = Platform::Thread_Create(FlushThreadFunc); + FlushThreadRunning = true; } - - void FlushThreadFunc() + else if (path[0] == '\0' && FlushThreadRunning) { - for (;;) - { - Platform::Sleep(100 * 1000); // 100ms - - if (!FlushThreadRunning) return; - - // We debounce for two seconds after last flush request to ensure that writing has finished. - if (TimeAtLastFlushRequest == 0 || difftime(time(NULL), TimeAtLastFlushRequest) < 2) - { - continue; - } - - FlushSecondaryBuffer(); - } - } - - void FlushSecondaryBuffer(u8* dst, s32 dstLength) - { - // When flushing to a file, there's no point in re-writing the exact same data. - if (!dst && !NeedsFlush()) return; - // When flushing to memory, we don't know if dst already has any data so we only check that we CAN flush. - if (dst && dstLength < SecondaryBufferLength) return; - - Platform::Mutex_Lock(SecondaryBufferLock); - if (dst) - { - memcpy(dst, SecondaryBuffer, SecondaryBufferLength); - } - else - { - FILE* f = Platform::OpenFile(Path, "wb"); - if (f) - { - printf("NDS SRAM: Written\n"); - fwrite(SecondaryBuffer, SecondaryBufferLength, 1, f); - fclose(f); - } - } - PreviousFlushVersion = FlushVersion; - TimeAtLastFlushRequest = 0; - Platform::Mutex_Unlock(SecondaryBufferLock); - } - - bool NeedsFlush() - { - return FlushVersion != PreviousFlushVersion; - } - - void UpdateBuffer(u8* src, s32 srcLength) - { - if (!src || srcLength != Length) return; - - // should we create a lock for the primary buffer? this method is not intended to be called from a secondary thread in the way Flush is - memcpy(Buffer, src, srcLength); - Platform::Mutex_Lock(SecondaryBufferLock); - memcpy(SecondaryBuffer, src, srcLength); - Platform::Mutex_Unlock(SecondaryBufferLock); - - PreviousFlushVersion = FlushVersion; + FlushThreadRunning = false; + Platform::Thread_Wait(FlushThread); + Platform::Thread_Free(FlushThread); } } + +void RequestFlush() +{ + Platform::Mutex_Lock(SecondaryBufferLock); + printf("NDS SRAM: Flush requested\n"); + memcpy(SecondaryBuffer, Buffer, Length); + FlushVersion++; + TimeAtLastFlushRequest = time(NULL); + Platform::Mutex_Unlock(SecondaryBufferLock); +} + +void FlushThreadFunc() +{ + for (;;) + { + Platform::Sleep(100 * 1000); // 100ms + + if (!FlushThreadRunning) return; + + // We debounce for two seconds after last flush request to ensure that writing has finished. + if (TimeAtLastFlushRequest == 0 || difftime(time(NULL), TimeAtLastFlushRequest) < 2) + { + continue; + } + + FlushSecondaryBuffer(); + } +} + +void FlushSecondaryBuffer(u8* dst, s32 dstLength) +{ + // When flushing to a file, there's no point in re-writing the exact same data. + if (!dst && !NeedsFlush()) return; + // When flushing to memory, we don't know if dst already has any data so we only check that we CAN flush. + if (dst && dstLength < SecondaryBufferLength) return; + + Platform::Mutex_Lock(SecondaryBufferLock); + if (dst) + { + memcpy(dst, SecondaryBuffer, SecondaryBufferLength); + } + else + { + FILE* f = Platform::OpenFile(Path, "wb"); + if (f) + { + printf("NDS SRAM: Written\n"); + fwrite(SecondaryBuffer, SecondaryBufferLength, 1, f); + fclose(f); + } + } + PreviousFlushVersion = FlushVersion; + TimeAtLastFlushRequest = 0; + Platform::Mutex_Unlock(SecondaryBufferLock); +} + +bool NeedsFlush() +{ + return FlushVersion != PreviousFlushVersion; +} + +void UpdateBuffer(u8* src, s32 srcLength) +{ + if (!src || srcLength != Length) return; + + // should we create a lock for the primary buffer? this method is not intended to be called from a secondary thread in the way Flush is + memcpy(Buffer, src, srcLength); + Platform::Mutex_Lock(SecondaryBufferLock); + memcpy(SecondaryBuffer, src, srcLength); + Platform::Mutex_Unlock(SecondaryBufferLock); + + PreviousFlushVersion = FlushVersion; +} + +}