From 4b78c92f4639235646271791cef17229e92f24e1 Mon Sep 17 00:00:00 2001 From: Daniel Oberhoff Date: Wed, 22 Nov 2023 22:18:03 +0100 Subject: [PATCH 1/3] use c++11 primitives for threads and locks --- CMakeLists.txt | 6 -- src/efsw/FileWatcherGeneric.cpp | 2 +- src/efsw/FileWatcherKqueue.cpp | 2 +- src/efsw/Lock.hpp | 14 +--- src/efsw/Mutex.cpp | 20 ----- src/efsw/Mutex.hpp | 25 +----- src/efsw/Thread.cpp | 41 ---------- src/efsw/Thread.hpp | 107 +++++++------------------ src/efsw/platform/platformimpl.hpp | 4 - src/efsw/platform/posix/MutexImpl.cpp | 28 ------- src/efsw/platform/posix/MutexImpl.hpp | 30 ------- src/efsw/platform/posix/ThreadImpl.cpp | 62 -------------- src/efsw/platform/posix/ThreadImpl.hpp | 39 --------- src/efsw/platform/win/MutexImpl.cpp | 25 ------ src/efsw/platform/win/MutexImpl.hpp | 33 -------- src/efsw/platform/win/ThreadImpl.cpp | 56 ------------- src/efsw/platform/win/ThreadImpl.hpp | 42 ---------- 17 files changed, 34 insertions(+), 502 deletions(-) delete mode 100644 src/efsw/Mutex.cpp delete mode 100644 src/efsw/Thread.cpp delete mode 100644 src/efsw/platform/posix/MutexImpl.cpp delete mode 100644 src/efsw/platform/posix/MutexImpl.hpp delete mode 100644 src/efsw/platform/posix/ThreadImpl.cpp delete mode 100644 src/efsw/platform/posix/ThreadImpl.hpp delete mode 100644 src/efsw/platform/win/MutexImpl.cpp delete mode 100644 src/efsw/platform/win/MutexImpl.hpp delete mode 100644 src/efsw/platform/win/ThreadImpl.cpp delete mode 100644 src/efsw/platform/win/ThreadImpl.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index e37438a..6c1f5a0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,10 +47,8 @@ set(EFSW_CPP_SOURCE src/efsw/FileWatcherGeneric.cpp src/efsw/FileWatcherImpl.cpp src/efsw/Log.cpp - src/efsw/Mutex.cpp src/efsw/String.cpp src/efsw/System.cpp - src/efsw/Thread.cpp src/efsw/Watcher.cpp src/efsw/WatcherGeneric.cpp ) @@ -77,16 +75,12 @@ endif() if(WIN32) list(APPEND EFSW_CPP_SOURCE src/efsw/platform/win/FileSystemImpl.cpp - src/efsw/platform/win/MutexImpl.cpp src/efsw/platform/win/SystemImpl.cpp - src/efsw/platform/win/ThreadImpl.cpp ) else() list(APPEND EFSW_CPP_SOURCE src/efsw/platform/posix/FileSystemImpl.cpp - src/efsw/platform/posix/MutexImpl.cpp src/efsw/platform/posix/SystemImpl.cpp - src/efsw/platform/posix/ThreadImpl.cpp ) endif() diff --git a/src/efsw/FileWatcherGeneric.cpp b/src/efsw/FileWatcherGeneric.cpp index 3f3c52e..e0d3930 100644 --- a/src/efsw/FileWatcherGeneric.cpp +++ b/src/efsw/FileWatcherGeneric.cpp @@ -101,7 +101,7 @@ void FileWatcherGeneric::removeWatch( WatchID watchid ) { void FileWatcherGeneric::watch() { if ( NULL == mThread ) { - mThread = new Thread( &FileWatcherGeneric::run, this ); + mThread = new Thread([this]{run();}); mThread->launch(); } } diff --git a/src/efsw/FileWatcherKqueue.cpp b/src/efsw/FileWatcherKqueue.cpp index 32ef3dc..41b4357 100644 --- a/src/efsw/FileWatcherKqueue.cpp +++ b/src/efsw/FileWatcherKqueue.cpp @@ -162,7 +162,7 @@ bool FileWatcherKqueue::isAddingWatcher() const { void FileWatcherKqueue::watch() { if ( NULL == mThread ) { - mThread = new Thread( &FileWatcherKqueue::run, this ); + mThread = new Thread([this]{run();}); mThread->launch(); } } diff --git a/src/efsw/Lock.hpp b/src/efsw/Lock.hpp index e8c522a..714f3aa 100644 --- a/src/efsw/Lock.hpp +++ b/src/efsw/Lock.hpp @@ -1,21 +1,11 @@ #ifndef EFSW_LOCK_HPP #define EFSW_LOCK_HPP +#include #include namespace efsw { - -/** Simple mutex class */ -class Lock { - public: - explicit Lock( Mutex& mutex ) : mMutex( mutex ) { mMutex.lock(); } - - ~Lock() { mMutex.unlock(); } - - private: - Mutex& mMutex; -}; - + using Lock = std::unique_lock; } // namespace efsw #endif diff --git a/src/efsw/Mutex.cpp b/src/efsw/Mutex.cpp deleted file mode 100644 index c961db1..0000000 --- a/src/efsw/Mutex.cpp +++ /dev/null @@ -1,20 +0,0 @@ -#include -#include - -namespace efsw { - -Mutex::Mutex() : mMutexImpl( new Platform::MutexImpl() ) {} - -Mutex::~Mutex() { - efSAFE_DELETE( mMutexImpl ); -} - -void Mutex::lock() { - mMutexImpl->lock(); -} - -void Mutex::unlock() { - mMutexImpl->unlock(); -} - -} // namespace efsw diff --git a/src/efsw/Mutex.hpp b/src/efsw/Mutex.hpp index d98ad17..1d8dca9 100644 --- a/src/efsw/Mutex.hpp +++ b/src/efsw/Mutex.hpp @@ -1,31 +1,10 @@ #ifndef EFSW_MUTEX_HPP #define EFSW_MUTEX_HPP -#include +#include namespace efsw { - -namespace Platform { -class MutexImpl; -} - -/** Simple mutex class */ -class Mutex { - public: - Mutex(); - - ~Mutex(); - - /** Lock the mutex */ - void lock(); - - /** Unlock the mutex */ - void unlock(); - - private: - Platform::MutexImpl* mMutexImpl; -}; - + using Mutex = std::mutex; } // namespace efsw #endif diff --git a/src/efsw/Thread.cpp b/src/efsw/Thread.cpp deleted file mode 100644 index cfa88b4..0000000 --- a/src/efsw/Thread.cpp +++ /dev/null @@ -1,41 +0,0 @@ -#include -#include - -namespace efsw { - -Thread::Thread() : mThreadImpl( NULL ), mEntryPoint( NULL ) {} - -Thread::~Thread() { - wait(); - - efSAFE_DELETE( mEntryPoint ); -} - -void Thread::launch() { - wait(); - - mThreadImpl = new Platform::ThreadImpl( this ); -} - -void Thread::wait() { - if ( mThreadImpl ) { - mThreadImpl->wait(); - - efSAFE_DELETE( mThreadImpl ); - } -} - -void Thread::terminate() { - if ( mThreadImpl ) { - mThreadImpl->terminate(); - - efSAFE_DELETE( mThreadImpl ); - } -} - -void Thread::run() { - if ( mEntryPoint ) - mEntryPoint->run(); -} - -} // namespace efsw diff --git a/src/efsw/Thread.hpp b/src/efsw/Thread.hpp index b60373c..5a5d470 100644 --- a/src/efsw/Thread.hpp +++ b/src/efsw/Thread.hpp @@ -2,99 +2,48 @@ #define EFSW_THREAD_HPP #include +#include +#include +#include namespace efsw { -namespace Platform { -class ThreadImpl; -} -namespace Private { -struct ThreadFunc; -} - /** @brief Thread manager class */ class Thread { public: - typedef void ( *FuncType )( void* ); - - template Thread( F function ); - - template Thread( F function, A argument ); - template Thread( void ( C::*function )(), C* object ); + Thread(std::function fun) + : mFun{std::move(fun)} + { + } - virtual ~Thread(); + ~Thread() + { + wait(); + } /** Launch the thread */ - virtual void launch(); + void launch() + { + if (!mThread) + mThread.reset(new std::thread{std::move(mFun)}); + } /** Wait the thread until end */ - void wait(); - - /** Terminate the thread */ - void terminate(); - - protected: - Thread(); - - private: - friend class Platform::ThreadImpl; - - /** The virtual function to run in the thread */ - virtual void run(); - - Platform::ThreadImpl* mThreadImpl; ///< OS-specific implementation of the thread - Private::ThreadFunc* mEntryPoint; ///< Abstraction of the function to run -}; - -//! NOTE: Taken from SFML2 threads -namespace Private { - -// Base class for abstract thread functions -struct ThreadFunc { - virtual ~ThreadFunc() {} - virtual void run() = 0; + void wait() + { + if (mThread) + { + mThread->join(); + mThread.reset(); + } + } +private: + + std::unique_ptr mThread; + std::function mFun; }; -// Specialization using a functor (including free functions) with no argument -template struct ThreadFunctor : ThreadFunc { - ThreadFunctor( T functor ) : m_functor( functor ) {} - virtual void run() { m_functor(); } - T m_functor; -}; - -// Specialization using a functor (including free functions) with one argument -template struct ThreadFunctorWithArg : ThreadFunc { - ThreadFunctorWithArg( F function, A arg ) : m_function( function ), m_arg( arg ) {} - virtual void run() { m_function( m_arg ); } - F m_function; - A m_arg; -}; - -// Specialization using a member function -template struct ThreadMemberFunc : ThreadFunc { - ThreadMemberFunc( void ( C::*function )(), C* object ) : - m_function( function ), m_object( object ) {} - virtual void run() { ( m_object->*m_function )(); } - void ( C::*m_function )(); - C* m_object; -}; - -} // namespace Private - -template -Thread::Thread( F functor ) : - mThreadImpl( NULL ), mEntryPoint( new Private::ThreadFunctor( functor ) ) {} - -template -Thread::Thread( F function, A argument ) : - mThreadImpl( NULL ), - mEntryPoint( new Private::ThreadFunctorWithArg( function, argument ) ) {} - -template -Thread::Thread( void ( C::*function )(), C* object ) : - mThreadImpl( NULL ), mEntryPoint( new Private::ThreadMemberFunc( function, object ) ) {} - } // namespace efsw #endif diff --git a/src/efsw/platform/platformimpl.hpp b/src/efsw/platform/platformimpl.hpp index 5442580..f494241 100644 --- a/src/efsw/platform/platformimpl.hpp +++ b/src/efsw/platform/platformimpl.hpp @@ -4,13 +4,9 @@ #include #if defined( EFSW_PLATFORM_POSIX ) -#include -#include #include #include #elif EFSW_PLATFORM == EFSW_PLATFORM_WIN32 -#include -#include #include #include #else diff --git a/src/efsw/platform/posix/MutexImpl.cpp b/src/efsw/platform/posix/MutexImpl.cpp deleted file mode 100644 index 2233798..0000000 --- a/src/efsw/platform/posix/MutexImpl.cpp +++ /dev/null @@ -1,28 +0,0 @@ -#include - -#if defined( EFSW_PLATFORM_POSIX ) - -namespace efsw { namespace Platform { - -MutexImpl::MutexImpl() { - pthread_mutexattr_t attributes; - pthread_mutexattr_init( &attributes ); - pthread_mutexattr_settype( &attributes, PTHREAD_MUTEX_RECURSIVE ); - pthread_mutex_init( &mMutex, &attributes ); -} - -MutexImpl::~MutexImpl() { - pthread_mutex_destroy( &mMutex ); -} - -void MutexImpl::lock() { - pthread_mutex_lock( &mMutex ); -} - -void MutexImpl::unlock() { - pthread_mutex_unlock( &mMutex ); -} - -}} // namespace efsw::Platform - -#endif diff --git a/src/efsw/platform/posix/MutexImpl.hpp b/src/efsw/platform/posix/MutexImpl.hpp deleted file mode 100644 index a33d827..0000000 --- a/src/efsw/platform/posix/MutexImpl.hpp +++ /dev/null @@ -1,30 +0,0 @@ -#ifndef EFSW_MUTEXIMPLPOSIX_HPP -#define EFSW_MUTEXIMPLPOSIX_HPP - -#include - -#if defined( EFSW_PLATFORM_POSIX ) - -#include - -namespace efsw { namespace Platform { - -class MutexImpl { - public: - MutexImpl(); - - ~MutexImpl(); - - void lock(); - - void unlock(); - - private: - pthread_mutex_t mMutex; -}; - -}} // namespace efsw::Platform - -#endif - -#endif diff --git a/src/efsw/platform/posix/ThreadImpl.cpp b/src/efsw/platform/posix/ThreadImpl.cpp deleted file mode 100644 index 0f96bca..0000000 --- a/src/efsw/platform/posix/ThreadImpl.cpp +++ /dev/null @@ -1,62 +0,0 @@ -#include -#include - -#if defined( EFSW_PLATFORM_POSIX ) - -#include -#include -#include - -namespace efsw { namespace Platform { - -ThreadImpl::ThreadImpl( efsw::Thread* owner ) : mIsActive( false ) { - mIsActive = pthread_create( &mThread, NULL, &ThreadImpl::entryPoint, owner ) == 0; - - if ( !mIsActive ) { - efDEBUG( "Failed to create thread\n" ); - } -} - -ThreadImpl::~ThreadImpl() { - terminate(); -} - -void ThreadImpl::wait() { - // Wait for the thread to finish, no timeout - if ( mIsActive ) { - assert( pthread_equal( pthread_self(), mThread ) == 0 ); - - mIsActive = pthread_join( mThread, NULL ) != 0; - } -} - -void ThreadImpl::terminate() { - if ( mIsActive ) { -#if !defined( __ANDROID__ ) && !defined( ANDROID ) - pthread_cancel( mThread ); -#else - pthread_kill( mThread, SIGUSR1 ); -#endif - - mIsActive = false; - } -} - -void* ThreadImpl::entryPoint( void* userData ) { -// Tell the thread to handle cancel requests immediatly -#ifdef PTHREAD_CANCEL_ASYNCHRONOUS - pthread_setcanceltype( PTHREAD_CANCEL_ASYNCHRONOUS, NULL ); -#endif - - // The Thread instance is stored in the user data - Thread* owner = static_cast( userData ); - - // Forward to the owner - owner->run(); - - return NULL; -} - -}} // namespace efsw::Platform - -#endif diff --git a/src/efsw/platform/posix/ThreadImpl.hpp b/src/efsw/platform/posix/ThreadImpl.hpp deleted file mode 100644 index 2e02f9a..0000000 --- a/src/efsw/platform/posix/ThreadImpl.hpp +++ /dev/null @@ -1,39 +0,0 @@ -#ifndef EFSW_THREADIMPLPOSIX_HPP -#define EFSW_THREADIMPLPOSIX_HPP - -#include - -#if defined( EFSW_PLATFORM_POSIX ) - -#include -#include - -namespace efsw { - -class Thread; - -namespace Platform { - -class ThreadImpl { - public: - explicit ThreadImpl( efsw::Thread* owner ); - - ~ThreadImpl(); - - void wait(); - - void terminate(); - - protected: - static void* entryPoint( void* userData ); - - pthread_t mThread; - Atomic mIsActive; -}; - -} // namespace Platform -} // namespace efsw - -#endif - -#endif diff --git a/src/efsw/platform/win/MutexImpl.cpp b/src/efsw/platform/win/MutexImpl.cpp deleted file mode 100644 index 62b7f83..0000000 --- a/src/efsw/platform/win/MutexImpl.cpp +++ /dev/null @@ -1,25 +0,0 @@ -#include - -#if EFSW_PLATFORM == EFSW_PLATFORM_WIN32 - -namespace efsw { namespace Platform { - -MutexImpl::MutexImpl() { - InitializeCriticalSection( &mMutex ); -} - -MutexImpl::~MutexImpl() { - DeleteCriticalSection( &mMutex ); -} - -void MutexImpl::lock() { - EnterCriticalSection( &mMutex ); -} - -void MutexImpl::unlock() { - LeaveCriticalSection( &mMutex ); -} - -}} // namespace efsw::Platform - -#endif diff --git a/src/efsw/platform/win/MutexImpl.hpp b/src/efsw/platform/win/MutexImpl.hpp deleted file mode 100644 index 7b06492..0000000 --- a/src/efsw/platform/win/MutexImpl.hpp +++ /dev/null @@ -1,33 +0,0 @@ -#ifndef EFSW_MUTEXIMPLWIN_HPP -#define EFSW_MUTEXIMPLWIN_HPP - -#include - -#if EFSW_PLATFORM == EFSW_PLATFORM_WIN32 - -#ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN -#endif -#include - -namespace efsw { namespace Platform { - -class MutexImpl { - public: - MutexImpl(); - - ~MutexImpl(); - - void lock(); - - void unlock(); - - private: - CRITICAL_SECTION mMutex; -}; - -}} // namespace efsw::Platform - -#endif - -#endif diff --git a/src/efsw/platform/win/ThreadImpl.cpp b/src/efsw/platform/win/ThreadImpl.cpp deleted file mode 100644 index 463934c..0000000 --- a/src/efsw/platform/win/ThreadImpl.cpp +++ /dev/null @@ -1,56 +0,0 @@ -#include -#include -#include - -#if EFSW_PLATFORM == EFSW_PLATFORM_WIN32 - -#include - -namespace efsw { namespace Platform { - -ThreadImpl::ThreadImpl( efsw::Thread* owner ) { - mThread = reinterpret_cast( - _beginthreadex( NULL, 0, &ThreadImpl::entryPoint, owner, 0, &mThreadId ) ); - - if ( !mThread ) { - efDEBUG( "Failed to create thread\n" ); - } -} - -ThreadImpl::~ThreadImpl() { - if ( mThread ) { - CloseHandle( mThread ); - } -} - -void ThreadImpl::wait() { - // Wait for the thread to finish, no timeout - if ( mThread ) { - assert( mThreadId != GetCurrentThreadId() ); // A thread cannot wait for itself! - - WaitForSingleObject( mThread, INFINITE ); - } -} - -void ThreadImpl::terminate() { - if ( mThread ) { - TerminateThread( mThread, 0 ); - } -} - -unsigned int __stdcall ThreadImpl::entryPoint( void* userData ) { - // The Thread instance is stored in the user data - Thread* owner = static_cast( userData ); - - // Forward to the owner - owner->run(); - - // Optional, but it is cleaner - _endthreadex( 0 ); - - return 0; -} - -}} // namespace efsw::Platform - -#endif diff --git a/src/efsw/platform/win/ThreadImpl.hpp b/src/efsw/platform/win/ThreadImpl.hpp deleted file mode 100644 index 455f24c..0000000 --- a/src/efsw/platform/win/ThreadImpl.hpp +++ /dev/null @@ -1,42 +0,0 @@ -#ifndef EFSW_THREADIMPLWIN_HPP -#define EFSW_THREADIMPLWIN_HPP - -#include - -#if EFSW_PLATFORM == EFSW_PLATFORM_WIN32 - -#ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN -#endif -#include -#include - -namespace efsw { - -class Thread; - -namespace Platform { - -class ThreadImpl { - public: - explicit ThreadImpl( efsw::Thread* owner ); - - ~ThreadImpl(); - - void wait(); - - void terminate(); - - protected: - static unsigned int __stdcall entryPoint( void* userData ); - - HANDLE mThread; - unsigned int mThreadId; -}; - -} // namespace Platform -} // namespace efsw - -#endif - -#endif From 24235a29ce6cbe11b9ad27ecb8c43a3ded77010a Mon Sep 17 00:00:00 2001 From: Daniel Oberhoff Date: Thu, 23 Nov 2023 08:11:30 +0100 Subject: [PATCH 2/3] fix thread uses in platform specific code --- src/efsw/FileWatcherInotify.cpp | 2 +- src/efsw/FileWatcherWin32.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/efsw/FileWatcherInotify.cpp b/src/efsw/FileWatcherInotify.cpp index 29be12b..132959b 100644 --- a/src/efsw/FileWatcherInotify.cpp +++ b/src/efsw/FileWatcherInotify.cpp @@ -244,7 +244,7 @@ void FileWatcherInotify::removeWatch( WatchID watchid ) { void FileWatcherInotify::watch() { if ( NULL == mThread ) { - mThread = new Thread( &FileWatcherInotify::run, this ); + mThread = new Thread([this]{run();}); mThread->launch(); } } diff --git a/src/efsw/FileWatcherWin32.cpp b/src/efsw/FileWatcherWin32.cpp index 2e26eeb..25d63ed 100644 --- a/src/efsw/FileWatcherWin32.cpp +++ b/src/efsw/FileWatcherWin32.cpp @@ -112,7 +112,7 @@ void FileWatcherWin32::removeWatch( WatcherStructWin32* watch ) { void FileWatcherWin32::watch() { if ( NULL == mThread ) { - mThread = new Thread( &FileWatcherWin32::run, this ); + mThread = new Thread([this]{run();}); mThread->launch(); } } From cf095334bceecdb495bf571ca4f890befa0c32c1 Mon Sep 17 00:00:00 2001 From: Daniel Oberhoff Date: Sat, 25 May 2024 09:14:27 +0200 Subject: [PATCH 3/3] use recursive mutex everywhere for now as before --- src/efsw/Mutex.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/efsw/Mutex.hpp b/src/efsw/Mutex.hpp index 1d8dca9..c1fca75 100644 --- a/src/efsw/Mutex.hpp +++ b/src/efsw/Mutex.hpp @@ -4,7 +4,7 @@ #include namespace efsw { - using Mutex = std::mutex; + using Mutex = std::recursive_mutex; } // namespace efsw #endif