From b61b59ae1b7cd4041ef0a815b7e96537418298c9 Mon Sep 17 00:00:00 2001 From: Aki Date: Tue, 5 Apr 2022 22:10:15 +0200 Subject: Fixed more potential locking problems in Campaign selection screen --- StarsEx/CmpSelectDlg.cpp | 89 +++++++++++++----------------------------------- StarsEx/CmpSelectDlg.h | 9 ++--- 2 files changed, 29 insertions(+), 69 deletions(-) (limited to 'StarsEx') 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 +#include +#include #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 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 lock(sync); + const std::shared_lock lock(sync); return !loading; } @@ -208,7 +209,7 @@ CmpSelectDlg::CanClose() void CmpSelectDlg::ShowNewCampaigns() { - const std::lock_guard 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 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 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 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 +#include +#include #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; -- cgit v1.1