From 2ae409ba0638d0ed33a36e30818a7cb44d35bd15 Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 24 Jun 2019 20:18:26 +0200 Subject: [PATCH 1/7] WinUpdater: Replaced PeekMessage with GetMessage - removes a busy loop in favour of a proper wait based message queue --- Source/Core/WinUpdater/WinUI.cpp | 38 ++++++++++++++------------------ 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/Source/Core/WinUpdater/WinUI.cpp b/Source/Core/WinUpdater/WinUI.cpp index 7f9a951bef..b187b77a42 100644 --- a/Source/Core/WinUpdater/WinUI.cpp +++ b/Source/Core/WinUpdater/WinUI.cpp @@ -24,7 +24,6 @@ HWND current_progressbar_handle = nullptr; ITaskbarList3* taskbar_list = nullptr; Common::Flag running; -Common::Flag request_stop; int GetWindowHeight(HWND hwnd) { @@ -33,6 +32,17 @@ int GetWindowHeight(HWND hwnd) return rect.bottom - rect.top; } + +LRESULT CALLBACK WindowProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) +{ + switch (uMsg) + { + case WM_DESTROY: + PostQuitMessage(0); + return 0; + } + return DefWindowProc(hwnd, uMsg, wParam, lParam); +} }; // namespace constexpr int PROGRESSBAR_FLAGS = WS_VISIBLE | WS_CHILD | PBS_SMOOTH | PBS_SMOOTHREVERSE; @@ -46,7 +56,7 @@ bool InitWindow() InitCommonControls(); WNDCLASS wndcl = {}; - wndcl.lpfnWndProc = DefWindowProcW; + wndcl.lpfnWndProc = WindowProc; wndcl.hbrBackground = GetSysColorBrush(COLOR_MENU); wndcl.lpszClassName = L"UPDATER"; @@ -124,14 +134,6 @@ bool InitWindow() return true; } -void Destroy() -{ - DestroyWindow(window_handle); - DestroyWindow(label_handle); - DestroyWindow(total_progressbar_handle); - DestroyWindow(current_progressbar_handle); -} - void SetTotalMarquee(bool marquee) { SetWindowLong(total_progressbar_handle, GWL_STYLE, @@ -199,7 +201,6 @@ void SetDescription(const std::string& text) void MessageLoop() { - request_stop.Clear(); running.Set(); if (!InitWindow()) @@ -211,19 +212,14 @@ void MessageLoop() SetTotalMarquee(true); SetCurrentMarquee(true); - while (!request_stop.IsSet()) + MSG msg; + while (GetMessage(&msg, nullptr, 0, 0)) { - MSG msg; - while (PeekMessage(&msg, window_handle, 0, 0, PM_REMOVE)) - { - TranslateMessage(&msg); - DispatchMessage(&msg); - } + TranslateMessage(&msg); + DispatchMessage(&msg); } running.Clear(); - - Destroy(); } void Init() @@ -237,7 +233,7 @@ void Stop() if (!running.IsSet()) return; - request_stop.Set(); + PostMessage(window_handle, WM_CLOSE, 0, 0); while (running.IsSet()) { From 3f1ba830e7718be59fe7a597b80764af6f083c79 Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 24 Jun 2019 20:49:18 +0200 Subject: [PATCH 2/7] UpdaterCommon: Remove manual UI::Stop() calls in favour of a scope guard to ensure it's called on all return paths --- Source/Core/UpdaterCommon/UpdaterCommon.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Source/Core/UpdaterCommon/UpdaterCommon.cpp b/Source/Core/UpdaterCommon/UpdaterCommon.cpp index 01c5149501..89a963609c 100644 --- a/Source/Core/UpdaterCommon/UpdaterCommon.cpp +++ b/Source/Core/UpdaterCommon/UpdaterCommon.cpp @@ -16,6 +16,7 @@ #include "Common/CommonPaths.h" #include "Common/FileUtil.h" #include "Common/HttpRequest.h" +#include "Common/ScopeGuard.h" #include "Common/StringUtil.h" #include "UpdaterCommon/UI.h" @@ -498,7 +499,6 @@ void FatalError(const std::string& message) UI::SetVisible(true); UI::Error(message); - UI::Stop(); } std::optional ParseManifest(const std::string& manifest) @@ -686,6 +686,7 @@ bool RunUpdater(std::vector args) UI::Init(); UI::SetVisible(false); + Common::ScopeGuard ui_guard{[] { UI::Stop(); }}; Options opts = std::move(*maybe_opts); if (opts.log_file) @@ -777,7 +778,5 @@ bool RunUpdater(std::vector args) UI::LaunchApplication(opts.binary_to_restart.value()); } - UI::Stop(); - return true; } From d355abaf0cb064963b9c6a4b99d5c70fe61ebf5a Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 24 Jun 2019 20:49:18 +0200 Subject: [PATCH 3/7] WinUpdater: Improved exit synchronization on Windows - now joins a thread instead of using flags to signal --- Source/Core/WinUpdater/WinUI.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/Source/Core/WinUpdater/WinUI.cpp b/Source/Core/WinUpdater/WinUI.cpp index b187b77a42..d65b00033d 100644 --- a/Source/Core/WinUpdater/WinUI.cpp +++ b/Source/Core/WinUpdater/WinUI.cpp @@ -12,7 +12,6 @@ #include #include -#include "Common/Flag.h" #include "Common/StringUtil.h" namespace @@ -23,7 +22,7 @@ HWND total_progressbar_handle = nullptr; HWND current_progressbar_handle = nullptr; ITaskbarList3* taskbar_list = nullptr; -Common::Flag running; +std::thread ui_thread; int GetWindowHeight(HWND hwnd) { @@ -201,12 +200,16 @@ void SetDescription(const std::string& text) void MessageLoop() { - running.Set(); - if (!InitWindow()) { - running.Clear(); MessageBox(nullptr, L"Window init failed!", L"", MB_ICONERROR); + // Destroying the parent (if exists) destroys all children windows + if (window_handle) + { + DestroyWindow(window_handle); + window_handle = nullptr; + } + return; } SetTotalMarquee(true); @@ -218,26 +221,19 @@ void MessageLoop() TranslateMessage(&msg); DispatchMessage(&msg); } - - running.Clear(); } void Init() { - std::thread thread(MessageLoop); - thread.detach(); + ui_thread = std::thread(MessageLoop); } void Stop() { - if (!running.IsSet()) - return; + if (window_handle) + PostMessage(window_handle, WM_CLOSE, 0, 0); - PostMessage(window_handle, WM_CLOSE, 0, 0); - - while (running.IsSet()) - { - } + ui_thread.join(); } void LaunchApplication(std::string path) From a00dfeecf0a7742366a32081d837fec1ebcead14 Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 24 Jun 2019 20:55:52 +0200 Subject: [PATCH 4/7] WinUpdater: Properly account for failure in WaitForPID --- Source/Core/WinUpdater/WinUI.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Source/Core/WinUpdater/WinUI.cpp b/Source/Core/WinUpdater/WinUI.cpp index d65b00033d..97f2da459b 100644 --- a/Source/Core/WinUpdater/WinUI.cpp +++ b/Source/Core/WinUpdater/WinUI.cpp @@ -251,8 +251,11 @@ void Sleep(int sleep) void WaitForPID(u32 pid) { HANDLE parent_handle = OpenProcess(SYNCHRONIZE, FALSE, static_cast(pid)); - WaitForSingleObject(parent_handle, INFINITE); - CloseHandle(parent_handle); + if (parent_handle) + { + WaitForSingleObject(parent_handle, INFINITE); + CloseHandle(parent_handle); + } } void SetVisible(bool visible) From 4b03790eda9b456c14328b42dab6829932d6128c Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 24 Jun 2019 21:02:30 +0200 Subject: [PATCH 5/7] Core: Fixup AutoUpdateChecker::TriggerUpdate on Windows: - Properly close handles if Updater.exe process spawns successfully - Fix STARTUPINFO sizeof typo --- Source/Core/UICommon/AutoUpdate.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Source/Core/UICommon/AutoUpdate.cpp b/Source/Core/UICommon/AutoUpdate.cpp index 00256ee174..0cca98b4b2 100644 --- a/Source/Core/UICommon/AutoUpdate.cpp +++ b/Source/Core/UICommon/AutoUpdate.cpp @@ -234,12 +234,16 @@ void AutoUpdateChecker::TriggerUpdate(const AutoUpdateChecker::NewVersionInforma INFO_LOG(COMMON, "Updater command line: %s", command_line.c_str()); #ifdef _WIN32 - STARTUPINFO sinfo = {sizeof(info)}; + STARTUPINFO sinfo = {sizeof(sinfo)}; sinfo.dwFlags = STARTF_FORCEOFFFEEDBACK; // No hourglass cursor after starting the process. PROCESS_INFORMATION pinfo; - if (!CreateProcessW(UTF8ToUTF16(reloc_updater_path).c_str(), - const_cast(UTF8ToUTF16(command_line).c_str()), nullptr, nullptr, - FALSE, 0, nullptr, nullptr, &sinfo, &pinfo)) + if (CreateProcessW(UTF8ToUTF16(reloc_updater_path).c_str(), UTF8ToUTF16(command_line).data(), + nullptr, nullptr, FALSE, 0, nullptr, nullptr, &sinfo, &pinfo)) + { + CloseHandle(pinfo.hThread); + CloseHandle(pinfo.hProcess); + } + else { ERROR_LOG(COMMON, "Could not start updater process: error=%d", GetLastError()); } From 94a19ca6702bceb6b70df0f6bac0445d7e68892e Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 24 Jun 2019 21:20:34 +0200 Subject: [PATCH 6/7] Qt/Updater: Fixed an assert on m_parent->close() (was called from a wrong thread) --- Source/Core/DolphinQt/Updater.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Updater.cpp b/Source/Core/DolphinQt/Updater.cpp index 788c5d8c0f..9783642ce0 100644 --- a/Source/Core/DolphinQt/Updater.cpp +++ b/Source/Core/DolphinQt/Updater.cpp @@ -95,6 +95,11 @@ void Updater::OnUpdateAvailable(const NewVersionInformation& info) AutoUpdateChecker::RestartMode::RESTART_AFTER_UPDATE); if (!later) - m_parent->close(); + { + RunOnObject(m_parent, [this] { + m_parent->close(); + return 0; + }); + } } } From baab660f1cde50d7ac3fdf936db6629a59f38477 Mon Sep 17 00:00:00 2001 From: Silent Date: Thu, 4 Jul 2019 21:33:11 +0200 Subject: [PATCH 7/7] WinUpdater: Removed MAX_PATH limitation on path to updater, also fixed a wrong size parameter --- Source/Core/WinUpdater/Main.cpp | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/Source/Core/WinUpdater/Main.cpp b/Source/Core/WinUpdater/Main.cpp index 0cb09cb966..9196805819 100644 --- a/Source/Core/WinUpdater/Main.cpp +++ b/Source/Core/WinUpdater/Main.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -32,6 +33,28 @@ std::vector CommandLineToUtf8Argv(PCWSTR command_line) LocalFree(tokenized); return argv; } + +std::optional GetModuleName(HINSTANCE hInstance) +{ + std::wstring name; + DWORD max_size = 50; // Start with space for 50 characters and grow if needed + name.resize(max_size); + + DWORD size; + while ((size = GetModuleFileNameW(hInstance, name.data(), max_size)) == max_size && + GetLastError() == ERROR_INSUFFICIENT_BUFFER) + { + max_size *= 2; + name.resize(max_size); + } + + if (size == 0) + { + return {}; + } + name.resize(size); + return name; +} }; // namespace int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PWSTR pCmdLine, int nCmdShow) @@ -64,15 +87,15 @@ int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PWSTR pCmdLine return 1; } - wchar_t path[MAX_PATH]; - if (GetModuleFileName(hInstance, path, sizeof(path)) == 0) + auto path = GetModuleName(hInstance); + if (!path) { MessageBox(nullptr, L"Failed to get updater filename.", L"Error", MB_ICONERROR); return 1; } // Relaunch the updater as administrator - ShellExecuteW(nullptr, L"runas", path, pCmdLine, NULL, SW_SHOW); + ShellExecuteW(nullptr, L"runas", path->c_str(), pCmdLine, NULL, SW_SHOW); return 0; }