From a870e0166aa7647bc16742b945c42e7521786362 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Wed, 18 Jan 2023 17:53:02 -0300 Subject: [PATCH] fix: added condition argument to wait calls in threadMain functions (#790) This addresses the issues reported by SonarCloud regarding the use of wait() without a condition argument in several threadMain functions. The changes made include the addition of a condition argument to the wait() calls in the threadMain functions in the DatabaseTasks, Scheduler and other classes. These changes ensure that the threads will not wait indefinitely and will only wake up when there is a task to be executed, thus avoiding any potential race conditions and deadlocks. --- src/database/databasetasks.cpp | 8 ++++++-- src/game/scheduling/scheduler.cpp | 4 +++- src/game/scheduling/tasks.cpp | 4 +++- src/otserv.cpp | 7 ++++++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/database/databasetasks.cpp b/src/database/databasetasks.cpp index 8d5e6cbfee3..ba7b317c098 100644 --- a/src/database/databasetasks.cpp +++ b/src/database/databasetasks.cpp @@ -48,7 +48,9 @@ void DatabaseTasks::threadMain() if (flushTasks) { flushSignal.notify_one(); } - taskSignal.wait(taskLockUnique); + taskSignal.wait(taskLockUnique, [this] { + return !tasks.empty(); + }); } if (!tasks.empty()) { @@ -102,7 +104,9 @@ void DatabaseTasks::flush() std::unique_lock guard{ taskLock }; if (!tasks.empty()) { flushTasks = true; - flushSignal.wait(guard); + flushSignal.wait(guard, [this] { + return !flushTasks; + }); flushTasks = false; } } diff --git a/src/game/scheduling/scheduler.cpp b/src/game/scheduling/scheduler.cpp index 539457ed95d..7aa26b50f33 100644 --- a/src/game/scheduling/scheduler.cpp +++ b/src/game/scheduling/scheduler.cpp @@ -19,7 +19,9 @@ void Scheduler::threadMain() eventLockUnique.lock(); if (eventList.empty()) { - eventSignal.wait(eventLockUnique); + eventSignal.wait(eventLockUnique, [this] { + return eventList.empty(); + }); } else { ret = eventSignal.wait_until(eventLockUnique, eventList.top()->getCycle()); } diff --git a/src/game/scheduling/tasks.cpp b/src/game/scheduling/tasks.cpp index 453ab4b607b..248b6aa79b0 100644 --- a/src/game/scheduling/tasks.cpp +++ b/src/game/scheduling/tasks.cpp @@ -33,7 +33,9 @@ void Dispatcher::threadMain() if (taskList.empty()) { //if the list is empty wait for signal - taskSignal.wait(taskLockUnique); + taskSignal.wait(taskLockUnique, [this] { + return taskList.empty(); + }); } if (!taskList.empty()) { diff --git a/src/otserv.cpp b/src/otserv.cpp index d6eb793b2f9..cfcbadbcfda 100644 --- a/src/otserv.cpp +++ b/src/otserv.cpp @@ -42,6 +42,7 @@ std::mutex g_loaderLock; std::condition_variable g_loaderSignal; std::unique_lock g_loaderUniqueLock(g_loaderLock); +bool g_loaderDone = false; /** *It is preferable to keep the close button off as it closes the server without saving (this can cause the player to lose items from houses and others informations, since windows automatically closes the process in five seconds, when forcing the close) @@ -217,7 +218,9 @@ int main(int argc, char* argv[]) { g_dispatcher().addTask(createTask(std::bind(mainLoader, argc, argv, &serviceManager))); - g_loaderSignal.wait(g_loaderUniqueLock); + g_loaderSignal.wait(g_loaderUniqueLock, [] { + return g_loaderDone; + }); if (serviceManager.is_running()) { SPDLOG_INFO("{} {}", g_configManager().getString(SERVER_NAME), @@ -383,5 +386,7 @@ void mainLoader(int, char*[], ServiceManager* services) { std::string url = g_configManager().getString(DISCORD_WEBHOOK_URL); webhook_send_message("Server is now online", "Server has successfully started.", WEBHOOK_COLOR_ONLINE, url); + g_loaderDone = true; + g_loaderSignal.notify_all(); }