From c2e74a93804f0ba4e7e87a95e8881bbe817aa0a4 Mon Sep 17 00:00:00 2001 From: lizzie Date: Thu, 9 Jul 2026 03:46:06 +0200 Subject: [PATCH] [common] remove DetachedTasks object usage (#4096) this was only ever used for announce room json on one single instance this object is pretty much a danger to society and shouldnt exist, especially since its: a) a singleton b) spawns objects out of thin air c) doesn't respect RAII and can outlive its parent objects (big no no) generally we shouldnt endorse objects like these in the future, but alas, it existed, now it has to begone additionally there is a similar object in KWorkerTaskManager but it's not as egregious as this one. check that this didnt break the usual announce room json logic Signed-off-by: lizzie Reviewed-on: https://git.eden-emu.dev/eden-emu/eden/pulls/4096 Reviewed-by: CamilleLaVey Reviewed-by: MaranBr --- src/android/app/src/main/jni/native.cpp | 2 -- src/android/app/src/main/jni/native.h | 2 -- src/common/CMakeLists.txt | 2 -- src/common/detached_tasks.cpp | 41 ------------------------- src/common/detached_tasks.h | 39 ----------------------- src/dedicated_room/yuzu_room.cpp | 6 +--- src/web_service/announce_room_json.cpp | 14 ++++----- src/web_service/announce_room_json.h | 6 ++++ src/yuzu/main.cpp | 13 ++------ src/yuzu_cmd/yuzu.cpp | 3 -- 10 files changed, 16 insertions(+), 112 deletions(-) delete mode 100644 src/common/detached_tasks.cpp delete mode 100644 src/common/detached_tasks.h diff --git a/src/android/app/src/main/jni/native.cpp b/src/android/app/src/main/jni/native.cpp index abfc428919..f40177ed03 100644 --- a/src/android/app/src/main/jni/native.cpp +++ b/src/android/app/src/main/jni/native.cpp @@ -39,7 +39,6 @@ #include "common/android/multiplayer/multiplayer.h" #include "common/android/android_common.h" #include "common/android/id_cache.h" -#include "common/detached_tasks.h" #include "common/dynamic_library.h" #include "common/fs/path_util.h" #include "common/logging.h" @@ -352,7 +351,6 @@ void EmulationSession::ShutdownEmulation() { if (m_load_result == Core::SystemResultStatus::Success) { m_system.DetachDebugger(); m_system.ShutdownMainProcess(); - m_detached_tasks.WaitForAllTasks(); m_load_result = Core::SystemResultStatus::ErrorNotInitialized; m_window.reset(); OnEmulationStopped(Core::SystemResultStatus::Success); diff --git a/src/android/app/src/main/jni/native.h b/src/android/app/src/main/jni/native.h index f2e5c2cfd6..6b171d5a66 100644 --- a/src/android/app/src/main/jni/native.h +++ b/src/android/app/src/main/jni/native.h @@ -6,7 +6,6 @@ #include #include "common/android/applets/software_keyboard.h" -#include "common/detached_tasks.h" #include "core/core.h" #include "core/file_sys/registered_cache.h" #include "core/hle/service/acc/profile_manager.h" @@ -75,7 +74,6 @@ private: // Core emulation Core::System m_system; InputCommon::InputSubsystem m_input_subsystem; - Common::DetachedTasks m_detached_tasks; Core::PerfStatsResults m_perf_stats{}; int m_shaders_building{0}; std::shared_ptr m_vfs; diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 7bb4c3aa4a..87c4642f04 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -40,8 +40,6 @@ add_library( container_hash.h demangle.cpp demangle.h - detached_tasks.cpp - detached_tasks.h device_power_state.cpp device_power_state.h div_ceil.h diff --git a/src/common/detached_tasks.cpp b/src/common/detached_tasks.cpp deleted file mode 100644 index f2ed795cc2..0000000000 --- a/src/common/detached_tasks.cpp +++ /dev/null @@ -1,41 +0,0 @@ -// SPDX-FileCopyrightText: 2018 Citra Emulator Project -// SPDX-License-Identifier: GPL-2.0-or-later - -#include -#include "common/assert.h" -#include "common/detached_tasks.h" - -namespace Common { - -DetachedTasks* DetachedTasks::instance = nullptr; - -DetachedTasks::DetachedTasks() { - ASSERT(instance == nullptr); - instance = this; -} - -void DetachedTasks::WaitForAllTasks() { - std::unique_lock lock{mutex}; - cv.wait(lock, [this]() { return count == 0; }); -} - -DetachedTasks::~DetachedTasks() { - WaitForAllTasks(); - - std::unique_lock lock{mutex}; - ASSERT(count == 0); - instance = nullptr; -} - -void DetachedTasks::AddTask(std::function task) { - std::unique_lock lock{instance->mutex}; - ++instance->count; - std::thread([task_{std::move(task)}]() { - task_(); - std::unique_lock thread_lock{instance->mutex}; - --instance->count; - std::notify_all_at_thread_exit(instance->cv, std::move(thread_lock)); - }).detach(); -} - -} // namespace Common diff --git a/src/common/detached_tasks.h b/src/common/detached_tasks.h deleted file mode 100644 index 416a2d7f3d..0000000000 --- a/src/common/detached_tasks.h +++ /dev/null @@ -1,39 +0,0 @@ -// SPDX-FileCopyrightText: 2018 Citra Emulator Project -// SPDX-License-Identifier: GPL-2.0-or-later - -#pragma once - -#include -#include - -namespace Common { - -/** - * A background manager which ensures that all detached task is finished before program exits. - * - * Some tasks, telemetry submission for example, prefer executing asynchronously and don't care - * about the result. These tasks are suitable for std::thread::detach(). However, this is unsafe if - * the task is launched just before the program exits (which is a common case for telemetry), so we - * need to block on these tasks on program exit. - * - * To make detached task safe, a single DetachedTasks object should be placed in the main(), and - * call WaitForAllTasks() after all program execution but before global/static variable destruction. - * Any potentially unsafe detached task should be executed via DetachedTasks::AddTask. - */ -class DetachedTasks { -public: - DetachedTasks(); - ~DetachedTasks(); - void WaitForAllTasks(); - - static void AddTask(std::function task); - -private: - static DetachedTasks* instance; - - std::condition_variable cv; - std::mutex mutex; - int count = 0; -}; - -} // namespace Common diff --git a/src/dedicated_room/yuzu_room.cpp b/src/dedicated_room/yuzu_room.cpp index d5b8c36141..824fd0f26c 100644 --- a/src/dedicated_room/yuzu_room.cpp +++ b/src/dedicated_room/yuzu_room.cpp @@ -24,7 +24,6 @@ #include #include "common/common_types.h" -#include "common/detached_tasks.h" #include "common/fs/file.h" #include "common/fs/fs.h" #include "common/fs/path_util.h" @@ -172,9 +171,7 @@ static void SaveBanList(const Network::Room::BanList& ban_list, const std::strin } /// Application entry point -void LaunchRoom(int argc, char** argv, bool called_by_option) -{ - Common::DetachedTasks detached_tasks; +void LaunchRoom(int argc, char** argv, bool called_by_option) { int option_index = 0; char* endarg; @@ -393,6 +390,5 @@ void LaunchRoom(int argc, char** argv, bool called_by_option) room->Destroy(); } Network::Shutdown(); - detached_tasks.WaitForAllTasks(); std::exit(0); } diff --git a/src/web_service/announce_room_json.cpp b/src/web_service/announce_room_json.cpp index b80caa49dc..c3a44257ef 100644 --- a/src/web_service/announce_room_json.cpp +++ b/src/web_service/announce_room_json.cpp @@ -6,7 +6,6 @@ #include #include -#include "common/detached_tasks.h" #include "common/logging.h" #include "web_service/announce_room_json.h" #include "web_service/web_backend.h" @@ -136,13 +135,14 @@ AnnounceMultiplayerRoom::RoomList RoomJson::GetRoomList() { void RoomJson::Delete() { if (room_id.empty()) { LOG_ERROR(WebService, "Room must be registered to be deleted"); - return; + } else { + // This jthread won't be destroyed until after the dtor has been ran + // Once the thread finishes it will stay resident on the vector -- destroyed and freed by dtor() + // this is still valid while in dtor, so... yeah + detached_tasks.emplace_back([this](std::stop_token stop_token) { + client.DeleteJson(fmt::format("/lobby/{}", room_id), "", false); + }); } - Common::DetachedTasks::AddTask([host_{this->host}, username_{this->username}, - token_{this->token}, room_id_{this->room_id}]() { - // create a new client here because the this->client might be destroyed. - Client{host_, username_, token_}.DeleteJson(fmt::format("/lobby/{}", room_id_), "", false); - }); } } // namespace WebService diff --git a/src/web_service/announce_room_json.h b/src/web_service/announce_room_json.h index 32c08858dd..a991325349 100644 --- a/src/web_service/announce_room_json.h +++ b/src/web_service/announce_room_json.h @@ -1,8 +1,13 @@ +// SPDX-FileCopyrightText: Copyright 2026 Eden Emulator Project +// SPDX-License-Identifier: GPL-3.0-or-later + // SPDX-FileCopyrightText: Copyright 2017 Citra Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include +#include #include #include #include "common/announce_multiplayer_room.h" @@ -30,6 +35,7 @@ public: void Delete() override; private: + std::vector detached_tasks; AnnounceMultiplayerRoom::Room room; Client client; std::string host; diff --git a/src/yuzu/main.cpp b/src/yuzu/main.cpp index 29a1f075fa..8cc979124c 100644 --- a/src/yuzu/main.cpp +++ b/src/yuzu/main.cpp @@ -8,9 +8,6 @@ #include #include "dedicated_room/yuzu_room.h" #endif - -#include - #ifdef __unix__ #include "qt_common/gui_settings.h" #endif @@ -104,8 +101,6 @@ int main(int argc, char* argv[]) { Breakpad::InstallCrashHandler(); #endif - Common::DetachedTasks detached_tasks; - // Init settings params QCoreApplication::setOrganizationName(QStringLiteral("eden")); QCoreApplication::setApplicationName(QStringLiteral("eden")); @@ -193,10 +188,6 @@ int main(int argc, char* argv[]) { // After settings have been loaded by GMainWindow, apply the filter main_window.show(); - app.connect(&app, &QGuiApplication::applicationStateChanged, &main_window, - &MainWindow::OnAppFocusStateChanged); - - int result = app.exec(); - detached_tasks.WaitForAllTasks(); - return result; + app.connect(&app, &QGuiApplication::applicationStateChanged, &main_window, &MainWindow::OnAppFocusStateChanged); + return app.exec(); } diff --git a/src/yuzu_cmd/yuzu.cpp b/src/yuzu_cmd/yuzu.cpp index d20c126159..6b33ca1b67 100644 --- a/src/yuzu_cmd/yuzu.cpp +++ b/src/yuzu_cmd/yuzu.cpp @@ -11,7 +11,6 @@ #include -#include "common/detached_tasks.h" #include "common/logging.h" #include "common/scm_rev.h" #include "common/settings.h" @@ -187,7 +186,6 @@ int main(int argc, char** argv) { Common::Log::Initialize(); Common::Log::SetColorConsoleBackendEnabled(true); Common::Log::Start(); - Common::DetachedTasks detached_tasks; int option_index = 0; #ifdef _WIN32 @@ -452,7 +450,6 @@ int main(int argc, char** argv) { system.DetachDebugger(); void(system.Pause()); system.ShutdownMainProcess(); - detached_tasks.WaitForAllTasks(); return 0; }