From 2a003a27b1045e4ed3f5d6bb4977b2d174a5c36c Mon Sep 17 00:00:00 2001 From: AbigailBuccaneer Date: Thu, 19 Apr 2018 20:29:06 +0100 Subject: [PATCH] Use C++11 threads, mutexes and condition variables --- src/CMakeLists.txt | 4 - src/app/app.cpp | 10 +- src/common/event.cpp | 19 +-- src/common/event.h | 5 +- src/common/thread/resource_owning_thread.h | 133 --------------------- src/common/thread/sdl_cond_wrapper.h | 62 ---------- src/common/thread/sdl_mutex_wrapper.h | 60 ---------- src/common/thread/thread.h | 77 ------------ src/common/thread/worker_thread.h | 49 ++++---- src/graphics/engine/engine.cpp | 6 +- src/sound/oalsound/alsound.cpp | 3 +- 11 files changed, 33 insertions(+), 395 deletions(-) delete mode 100644 src/common/thread/resource_owning_thread.h delete mode 100644 src/common/thread/sdl_cond_wrapper.h delete mode 100644 src/common/thread/sdl_mutex_wrapper.h delete mode 100644 src/common/thread/thread.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e0857506..271aa75b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -150,10 +150,6 @@ set(BASE_SOURCES common/singleton.h common/stringutils.cpp common/stringutils.h - common/thread/resource_owning_thread.h - common/thread/sdl_cond_wrapper.h - common/thread/sdl_mutex_wrapper.h - common/thread/thread.h common/thread/worker_thread.h graphics/core/color.cpp graphics/core/color.h diff --git a/src/app/app.cpp b/src/app/app.cpp index ef929e43..4ef861f6 100644 --- a/src/app/app.cpp +++ b/src/app/app.cpp @@ -36,8 +36,6 @@ #include "common/system/system.h" -#include "common/thread/thread.h" - #include "graphics/core/nulldevice.h" #include "graphics/opengl/glutil.h" @@ -60,6 +58,7 @@ #include #include #include +#include char CApplication::m_languageLocale[] = { 0 }; @@ -671,8 +670,7 @@ bool CApplication::Create() // Create the robot application. m_controller = MakeUnique(); - CThread musicLoadThread([this]() - { + std::thread{[this]() { GetLogger()->Debug("Cache sounds...\n"); SystemTimeStamp* musicLoadStart = m_systemUtils->CreateTimeStamp(); m_systemUtils->GetCurrentTimeStamp(musicLoadStart); @@ -683,9 +681,7 @@ bool CApplication::Create() m_systemUtils->GetCurrentTimeStamp(musicLoadEnd); float musicLoadTime = m_systemUtils->TimeStampDiff(musicLoadStart, musicLoadEnd, STU_MSEC); GetLogger()->Debug("Sound loading took %.2f ms\n", musicLoadTime); - }, - "Sound loading thread"); - musicLoadThread.Start(); + }}.detach(); if (m_runSceneCategory == LevelCategory::Max) m_controller->StartApp(); diff --git a/src/common/event.cpp b/src/common/event.cpp index 36f2bcb5..510cee7e 100644 --- a/src/common/event.cpp +++ b/src/common/event.cpp @@ -563,8 +563,7 @@ std::string ParseEventType(EventType eventType) CEventQueue::CEventQueue() - : m_mutex{}, - m_fifo(), + : m_fifo(), m_head{0}, m_tail{0}, m_total{0} @@ -582,15 +581,13 @@ bool CEventQueue::IsEmpty() Else, adds the event to the queue and returns \c true. */ bool CEventQueue::AddEvent(Event&& event) { - bool result{}; - - SDL_LockMutex(*m_mutex); + std::lock_guard lock{m_mutex}; if (m_total >= MAX_EVENT_QUEUE) { GetLogger()->Warn("Event queue flood!\n"); - result = false; + return false; } else { @@ -601,19 +598,15 @@ bool CEventQueue::AddEvent(Event&& event) m_total++; - result = true; + return true; } - - SDL_UnlockMutex(*m_mutex); - - return result; } Event CEventQueue::GetEvent() { Event event; - SDL_LockMutex(*m_mutex); + std::lock_guard lock{m_mutex}; if (m_head == m_tail) { @@ -630,7 +623,5 @@ Event CEventQueue::GetEvent() } - SDL_UnlockMutex(*m_mutex); - return event; } diff --git a/src/common/event.h b/src/common/event.h index fe9b6448..00ad33cb 100644 --- a/src/common/event.h +++ b/src/common/event.h @@ -27,12 +27,11 @@ #include "common/key.h" #include "common/make_unique.h" -#include "common/thread/sdl_mutex_wrapper.h" - #include "math/point.h" #include "math/vector.h" #include +#include /** \enum EventType @@ -888,7 +887,7 @@ public: Event GetEvent(); protected: - CSDLMutexWrapper m_mutex; + std::mutex m_mutex; Event m_fifo[MAX_EVENT_QUEUE]; int m_head; int m_tail; diff --git a/src/common/thread/resource_owning_thread.h b/src/common/thread/resource_owning_thread.h deleted file mode 100644 index 995cf4cb..00000000 --- a/src/common/thread/resource_owning_thread.h +++ /dev/null @@ -1,133 +0,0 @@ -/* - * This file is part of the Colobot: Gold Edition source code - * Copyright (C) 2001-2018, Daniel Roux, EPSITEC SA & TerranovaTeam - * http://epsitec.ch; http://colobot.info; http://github.com/colobot - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://gnu.org/licenses - */ - -#pragma once - -#include "common/thread/sdl_cond_wrapper.h" -#include "common/thread/sdl_mutex_wrapper.h" - -#include - -#include -#include - -/** - * \class CResourceOwningThread - * \brief Wrapper around SDL thread allowing passing of resources in safe manner - * - * This class is a workaround for passing ownership of resources in a safe - * manner to newly created threads. It takes a pointer to a function to call - * in new thread and a unique_ptr to resource which is to be passed to the new thread. - * - * This is how it works: - * - in main thread: create a new thread passing to it a special temporary context, - * - in main thread: wait for synchronization signal that the ownership was passed, - * - in new thread: acquire the resource from the context - * - in new thread: signal back to main thread that the resource was acquired, - * - in main thread: clean up temporary context and exit - * - in new thread: run the specified function with the acquired resource. - * - * It's a bit complicated, but that's the safe (thread-safe and exception-safe) - * way of doing this. - */ -template -class CResourceOwningThread -{ -public: - using ResourceUPtr = std::unique_ptr; - using ThreadFunctionPtr = void(*)(ResourceUPtr); - - CResourceOwningThread(ThreadFunctionPtr threadFunction, ResourceUPtr resource, std::string name = "") - : m_threadFunction(threadFunction), - m_resource(std::move(resource)), - m_name(name) - {} - - ~CResourceOwningThread() - { - SDL_DetachThread(m_thread); - } - - void Start() - { - CSDLMutexWrapper mutex; - CSDLCondWrapper cond; - bool condition = false; - - ThreadData data; - data.resource = std::move(m_resource); - data.threadFunction = m_threadFunction; - data.mutex = &mutex; - data.cond = &cond; - data.condition = &condition; - - SDL_LockMutex(*mutex); - - m_thread = SDL_CreateThread(Run, !m_name.empty() ? m_name.c_str() : nullptr, reinterpret_cast(&data)); - - while (!condition) - { - SDL_CondWait(*cond, *mutex); - } - - SDL_UnlockMutex(*mutex); - } - - void Join() - { - if (m_thread == nullptr) return; - SDL_WaitThread(m_thread, nullptr); - m_thread = nullptr; - } - -private: - static int Run(void* data) - { - ThreadFunctionPtr threadFunction = nullptr; - ResourceUPtr resource; - - ThreadData* threadData = reinterpret_cast(data); - SDL_LockMutex(**threadData->mutex); - - threadFunction = threadData->threadFunction; - resource = std::move(threadData->resource); - - *threadData->condition = true; - SDL_CondSignal(**threadData->cond); - SDL_UnlockMutex(**threadData->mutex); - - threadFunction(std::move(resource)); - return 0; - } - -private: - struct ThreadData - { - ResourceUPtr resource; - CSDLMutexWrapper* mutex = nullptr; - CSDLCondWrapper* cond = nullptr; - bool* condition = nullptr; - ThreadFunctionPtr threadFunction = nullptr; - }; - - ThreadFunctionPtr m_threadFunction; - ResourceUPtr m_resource; - std::string m_name; - SDL_Thread* m_thread = nullptr; -}; diff --git a/src/common/thread/sdl_cond_wrapper.h b/src/common/thread/sdl_cond_wrapper.h deleted file mode 100644 index 1be681cf..00000000 --- a/src/common/thread/sdl_cond_wrapper.h +++ /dev/null @@ -1,62 +0,0 @@ -/* - * This file is part of the Colobot: Gold Edition source code - * Copyright (C) 2001-2018, Daniel Roux, EPSITEC SA & TerranovaTeam - * http://epsitec.ch; http://colobot.info; http://github.com/colobot - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://gnu.org/licenses - */ - -#pragma once - -#include "common/thread/sdl_mutex_wrapper.h" - -#include - -/** - * \class CSDLCondWrapper - * \brief Wrapper for safe creation/deletion of SDL_cond - */ -class CSDLCondWrapper -{ -public: - CSDLCondWrapper() - : m_cond(SDL_CreateCond()) - {} - - ~CSDLCondWrapper() - { - SDL_DestroyCond(m_cond); - } - - CSDLCondWrapper(const CSDLCondWrapper&) = delete; - CSDLCondWrapper& operator=(const CSDLCondWrapper&) = delete; - - SDL_cond* operator*() - { - return m_cond; - } - - void Signal() - { - SDL_CondSignal(m_cond); - } - - void Wait(SDL_mutex* mutex) - { - SDL_CondWait(m_cond, mutex); - } - -private: - SDL_cond* m_cond; -}; diff --git a/src/common/thread/sdl_mutex_wrapper.h b/src/common/thread/sdl_mutex_wrapper.h deleted file mode 100644 index 476ddcae..00000000 --- a/src/common/thread/sdl_mutex_wrapper.h +++ /dev/null @@ -1,60 +0,0 @@ -/* - * This file is part of the Colobot: Gold Edition source code - * Copyright (C) 2001-2018, Daniel Roux, EPSITEC SA & TerranovaTeam - * http://epsitec.ch; http://colobot.info; http://github.com/colobot - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://gnu.org/licenses - */ - -#pragma once - -#include - -/** - * \class CSDLMutexWrapper - * \brief Wrapper for safe creation/deletion of SDL_mutex - */ -class CSDLMutexWrapper -{ -public: - CSDLMutexWrapper() - : m_mutex(SDL_CreateMutex()) - {} - - ~CSDLMutexWrapper() - { - SDL_DestroyMutex(m_mutex); - } - - CSDLMutexWrapper(const CSDLMutexWrapper&) = delete; - CSDLMutexWrapper& operator=(const CSDLMutexWrapper&) = delete; - - SDL_mutex* operator*() - { - return m_mutex; - } - - void Lock() - { - SDL_LockMutex(m_mutex); - } - - void Unlock() - { - SDL_UnlockMutex(m_mutex); - } - -private: - SDL_mutex* m_mutex; -}; diff --git a/src/common/thread/thread.h b/src/common/thread/thread.h deleted file mode 100644 index 59433ae3..00000000 --- a/src/common/thread/thread.h +++ /dev/null @@ -1,77 +0,0 @@ -/* - * This file is part of the Colobot: Gold Edition source code - * Copyright (C) 2001-2018, Daniel Roux, EPSITEC SA & TerranovaTeam - * http://epsitec.ch; http://colobot.info; http://github.com/colobot - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://gnu.org/licenses - */ - -#pragma once - -#include "common/make_unique.h" - -#include "common/thread/resource_owning_thread.h" - -#include -#include -#include - -/** - * \class CThread - * \brief Wrapper for using SDL_thread with std::function - */ -class CThread -{ -public: - using ThreadFunctionPtr = std::function; - -private: - struct ThreadData - { - ThreadFunctionPtr func; - }; - -public: - CThread(ThreadFunctionPtr func, std::string name = "") - : m_func(std::move(func)) - , m_name(name) - {} - - void Start() - { - std::unique_ptr data = MakeUnique(); - data->func = m_func; - m_thread = MakeUnique>(Run, std::move(data), m_name); - m_thread->Start(); - } - - void Join() - { - if (!m_thread) return; - m_thread->Join(); - } - - CThread(const CThread&) = delete; - CThread& operator=(const CThread&) = delete; - -private: - static void Run(std::unique_ptr data) - { - data->func(); - } - - std::unique_ptr> m_thread; - ThreadFunctionPtr m_func; - std::string m_name; -}; diff --git a/src/common/thread/worker_thread.h b/src/common/thread/worker_thread.h index ad127556..44b6bf41 100644 --- a/src/common/thread/worker_thread.h +++ b/src/common/thread/worker_thread.h @@ -21,13 +21,12 @@ #include "common/make_unique.h" -#include "common/thread/sdl_cond_wrapper.h" -#include "common/thread/sdl_mutex_wrapper.h" -#include "common/thread/thread.h" - +#include #include -#include +#include #include +#include +#include /** * \class CWorkerThread @@ -39,27 +38,23 @@ public: using ThreadFunctionPtr = std::function; public: - CWorkerThread(std::string name = "") - : m_thread(std::bind(&CWorkerThread::Run, this), name) - { - m_thread.Start(); - } + CWorkerThread() : m_thread{&CWorkerThread::Run, this} {} ~CWorkerThread() { - m_mutex.Lock(); - m_running = false; - m_cond.Signal(); - m_mutex.Unlock(); - m_thread.Join(); + { + std::lock_guard lock{m_mutex}; + m_running = false; + m_cond.notify_one(); + } + m_thread.join(); } - void Start(ThreadFunctionPtr func) + void Start(ThreadFunctionPtr&& func) { - m_mutex.Lock(); + std::lock_guard lock{m_mutex}; m_queue.push(func); - m_cond.Signal(); - m_mutex.Unlock(); + m_cond.notify_one(); } CWorkerThread(const CWorkerThread&) = delete; @@ -68,25 +63,21 @@ public: private: void Run() { - m_mutex.Lock(); + auto lock = std::unique_lock(m_mutex); while (true) { - while (m_queue.empty() && m_running) - { - m_cond.Wait(*m_mutex); - } + m_cond.wait(lock, [&]() { return !m_running || !m_queue.empty(); }); if (!m_running) break; - ThreadFunctionPtr func = m_queue.front(); + ThreadFunctionPtr func = std::move(m_queue.front()); m_queue.pop(); func(); } - m_mutex.Unlock(); } - CThread m_thread; - CSDLMutexWrapper m_mutex; - CSDLCondWrapper m_cond; + std::thread m_thread; + std::mutex m_mutex; + std::condition_variable m_cond; bool m_running = true; std::queue m_queue; }; diff --git a/src/graphics/engine/engine.cpp b/src/graphics/engine/engine.cpp index e489b1e8..0f86e6fd 100644 --- a/src/graphics/engine/engine.cpp +++ b/src/graphics/engine/engine.cpp @@ -32,8 +32,6 @@ #include "common/system/system.h" -#include "common/thread/resource_owning_thread.h" - #include "graphics/core/device.h" #include "graphics/core/framebuffer.h" @@ -63,6 +61,7 @@ #include #include #include +#include // Graphics module namespace namespace Gfx @@ -500,8 +499,7 @@ void CEngine::WriteScreenShot(const std::string& fileName) data->fileName = fileName; - CResourceOwningThread thread(CEngine::WriteScreenShotThread, std::move(data), "WriteScreenShot thread"); - thread.Start(); + std::thread{&CEngine::WriteScreenShotThread, std::move(data)}.detach(); } void CEngine::WriteScreenShotThread(std::unique_ptr data) diff --git a/src/sound/oalsound/alsound.cpp b/src/sound/oalsound/alsound.cpp index e4d457d3..c682e8b7 100644 --- a/src/sound/oalsound/alsound.cpp +++ b/src/sound/oalsound/alsound.cpp @@ -32,8 +32,7 @@ CALSound::CALSound() m_musicVolume(1.0f), m_channelsLimit(2048), m_device{}, - m_context{}, - m_thread("Music loading thread") + m_context{} { }