Fixed more potential locking problems in Campaign selection screen

2 files changed, 29 insertions, 69 deletions

diff --git a/StarsEx/CmpSelectDlg.cpp b/StarsEx/CmpSelectDlg.cpp
index b2261ac..5e29cd0 100644
--- a/StarsEx/CmpSelectDlg.cpp
+++ b/StarsEx/CmpSelectDlg.cpp
@@ -11,7 +11,8 @@
     Mission Select Dialog Active Window class
 */
 
-#include <mutex>
+#include <shared_mutex>
+#include <thread>
 
 #include "CmpSelectDlg.h"
 #include "ConfirmDlg.h"
@@ -54,7 +55,7 @@ CmpSelectDlg::CmpSelectDlg(Screen* s, FormDef& def, MenuScreen* mgr)
 lst_campaigns(0), btn_new(0), btn_saved(0), btn_delete(0),
 btn_accept(0), btn_cancel(0), description(0), stars(0), campaign(0),
 selected_mission(-1), show_saved(false), loading(false),
-loaded(false), hproc(0)
+loaded(false)
 {
     stars = Starshatter::GetInstance();
     select_msg = ContentBundle::GetInstance()->GetText("CmpSelectDlg.select_msg");
@@ -120,7 +121,7 @@ CmpSelectDlg::ExecFrame()
         OnAccept(0);
     }
 
-    const std::lock_guard<std::mutex> lock(sync);
+    const std::unique_lock lock(sync);
 
     if (loaded) {
         loaded = false;
@@ -199,7 +200,7 @@ CmpSelectDlg::ExecFrame()
 bool
 CmpSelectDlg::CanClose()
 {
-    const std::lock_guard<std::mutex> lock(sync);
+    const std::shared_lock lock(sync);
     return !loading;
 }
 
@@ -208,7 +209,7 @@ CmpSelectDlg::CanClose()
 void
 CmpSelectDlg::ShowNewCampaigns()
 {
-    const std::lock_guard<std::mutex> lock(sync);
+    const std::shared_lock lock(sync);
 
     if (loading && description) {
         description->SetText(ContentBundle::GetInstance()->GetText("CmpSelectDlg.already-loading"));
@@ -285,7 +286,7 @@ CmpSelectDlg::ShowNewCampaigns()
 void
 CmpSelectDlg::ShowSavedCampaigns()
 {
-    const std::lock_guard<std::mutex> lock(sync);
+    const std::shared_lock lock(sync);
 
     if (loading && description) {
         description->SetText(ContentBundle::GetInstance()->GetText("CmpSelectDlg.already-loading"));
@@ -333,9 +334,9 @@ CmpSelectDlg::ShowSavedCampaigns()
 void
 CmpSelectDlg::OnCampaignSelect(AWEvent* event)
 {
-    if (description && lst_campaigns) {
-        const std::lock_guard<std::mutex> lock(sync);
+    const std::shared_lock lock(sync);
 
+    if (description && lst_campaigns) {
         if (loading) {
             description->SetText(ContentBundle::GetInstance()->GetText("CmpSelectDlg.already-loading"));
             Button::PlaySound(Button::SND_REJECT);
@@ -486,7 +487,7 @@ CmpSelectDlg::OnConfirmDelete(AWEvent* event)
 void
 CmpSelectDlg::OnAccept(AWEvent* event)
 {
-    const std::lock_guard<std::mutex> lock(sync);
+    const std::shared_lock lock(sync);
 
     if (loading)
     return;
@@ -510,69 +511,32 @@ CmpSelectDlg::OnCancel(AWEvent* event)
 
 // +--------------------------------------------------------------------+
 
-DWORD WINAPI CmpSelectDlgLoadProc(LPVOID link);
-
 void
 CmpSelectDlg::StartLoadProc()
 {
-    if (hproc != 0) {
-        DWORD result = 0;
-        GetExitCodeThread(hproc, &result);
-
-        if (result != STILL_ACTIVE) {
-            CloseHandle(hproc);
-            hproc = 0;
-        }
-        else {
+    if (hproc.joinable()) {
+        const std::shared_lock lock(sync);
+        if (loading)
             return;
-        }
+        else
+            hproc.join();
     }
-
-    if (hproc == 0) {
-        campaign = 0;
-        loading = true;
-        loaded  = false;
-
-        if (description)
+    campaign = nullptr;
+    loading = true;
+    loaded = false;
+    if (description)
         description->SetText(ContentBundle::GetInstance()->GetText("CmpSelectDlg.loading"));
-
-        DWORD thread_id = 0;
-        hproc = CreateThread(0, 4096, CmpSelectDlgLoadProc, (LPVOID) this, 0, &thread_id);
-
-        if (hproc == 0) {
-            static int report = 10;
-            if (report > 0) {
-                ::Print("WARNING: CmpSelectDlg() failed to create thread (err=%08x)\n", GetLastError());
-                report--;
-
-                if (report == 0)
-                ::Print("         Further warnings of this type will be supressed.\n");
-            }
-        }
-    }
+    hproc = std::thread([this]{ LoadProc(); });
 }
 
 void
 CmpSelectDlg::StopLoadProc()
 {
-    if (hproc != 0) {
-        WaitForSingleObject(hproc, 2500);
-        CloseHandle(hproc);
-        hproc = 0;
-    }
+    if (hproc.joinable())
+        hproc.join();
 }
 
-DWORD WINAPI CmpSelectDlgLoadProc(LPVOID link)
-{
-    CmpSelectDlg* dlg = (CmpSelectDlg*) link;
-
-    if (dlg)
-    return dlg->LoadProc();
-
-    return (DWORD) E_POINTER;
-}
-
-DWORD
+void
 CmpSelectDlg::LoadProc()
 {
     Campaign* c = 0;
@@ -594,15 +558,10 @@ CmpSelectDlg::LoadProc()
         c = savegame.GetCampaign();
     }
 
-    sync.lock();
-
+    std::unique_lock lock(sync);
     loading = false;
     loaded  = true;
     campaign = c;
-
-    sync.unlock();
-
-    return 0;
 }
 
 // +--------------------------------------------------------------------+
diff --git a/StarsEx/CmpSelectDlg.h b/StarsEx/CmpSelectDlg.h
index 093e21d..ee52425 100644
--- a/StarsEx/CmpSelectDlg.h
+++ b/StarsEx/CmpSelectDlg.h
@@ -14,7 +14,8 @@
 #ifndef CmpSelectDlg_h
 #define CmpSelectDlg_h
 
-#include <mutex>
+#include <shared_mutex>
+#include <thread>
 
 #include "Types.h"
 #include "FormWindow.h"
@@ -53,7 +54,7 @@ public:
     virtual void      OnAccept(AWEvent* event);
     virtual void      OnCancel(AWEvent* event);
 
-    virtual DWORD     LoadProc();
+    virtual void      LoadProc();
 
 protected:
     virtual void      StartLoadProc();
@@ -76,8 +77,8 @@ protected:
     Starshatter*      stars;
     Campaign*         campaign;
     int               selected_mission;
-    HANDLE            hproc;
-    std::mutex        sync;
+    std::thread       hproc;
+    std::shared_mutex sync;
     bool              loading;
     bool              loaded;
     Text              load_file;