avoid leaking threads in NDSCart_SRAMManager

also atomics

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);

src/GPU3D_Soft.h

@@ -21,6 +21,7 @@
 #include "GPU3D.h"
 #include "Platform.h"
 #include <thread>
+#include <atomic>
 
 namespace GPU3D
 {
@@ -506,8 +507,8 @@ private:
 
     bool Threaded;
     Platform::Thread* RenderThread;
-    bool RenderThreadRunning;
+    std::atomic_bool RenderThreadRunning;
-    bool RenderThreadRendering;
+    std::atomic_bool RenderThreadRendering;
     Platform::Semaphore* Sema_RenderStart;
     Platform::Semaphore* Sema_RenderDone;
     Platform::Semaphore* Sema_ScanlineCount;

src/NDSCart_SRAMManager.cpp

@@ -20,156 +20,165 @@
 #include <string.h>
 #include <unistd.h>
 #include <time.h>
+#include <atomic>
 #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;
+char Path[1024];
-    u32 Length;
 
-    u8* SecondaryBuffer;
+u8* Buffer;
-    u32 SecondaryBufferLength;
+u32 Length;
-    
-    time_t TimeAtLastFlushRequest;
 
-    // We keep versions in case the user closes the application before
+u8* SecondaryBuffer;
-    // a flush cycle is finished.
+u32 SecondaryBufferLength;
-    u32 PreviousFlushVersion;
-    u32 FlushVersion;
 
-    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();
+        FlushThreadRunning = false;
-
+        Platform::Thread_Wait(FlushThread);
-        return true;
+        Platform::Thread_Free(FlushThread);
-    }
-
-    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.
         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);
+        FlushThread = Platform::Thread_Create(FlushThreadFunc);
-        printf("NDS SRAM: Flush requested\n");
+        FlushThreadRunning = true;
-        memcpy(SecondaryBuffer, Buffer, Length);
-        FlushVersion++;
-        TimeAtLastFlushRequest = time(NULL);
-        Platform::Mutex_Unlock(SecondaryBufferLock);
     }
-    
+    else if (path[0] == '\0' && FlushThreadRunning)
-    void FlushThreadFunc()
     {
-        for (;;)
+        FlushThreadRunning = false;
-        {
+        Platform::Thread_Wait(FlushThread);
-            Platform::Sleep(100 * 1000); // 100ms
+        Platform::Thread_Free(FlushThread);
-
-            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;
     }
 }
+
+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;
+}
+
+}