From 76ccef09a1e590f4ad6cb1339f263396e1618f50 Mon Sep 17 00:00:00 2001 From: Kerstin Keller Date: Wed, 24 Jan 2024 11:40:14 +0100 Subject: [PATCH 1/2] [Core] Fix: proper destructors / rule of 5 for everything involving std::thread. --- ecal/core/include/ecal/ecal_timed_cb.h | 10 +++++++++- ecal/core/src/io/shm/ecal_memfile_pool.cpp | 5 ++++- ecal/core/src/io/shm/ecal_memfile_pool.h | 5 +++++ ecal/core/src/time/ecal_timer.cpp | 4 ++++ ecal/core/src/util/ecal_thread.h | 10 ++++++++++ 5 files changed, 32 insertions(+), 2 deletions(-) diff --git a/ecal/core/include/ecal/ecal_timed_cb.h b/ecal/core/include/ecal/ecal_timed_cb.h index 5581b6d518..fa4af6505b 100644 --- a/ecal/core/include/ecal/ecal_timed_cb.h +++ b/ecal/core/include/ecal/ecal_timed_cb.h @@ -68,6 +68,11 @@ namespace eCAL **/ virtual ~CTimedCB() { Stop(); } + CTimedCB(const CTimedCB&) = delete; + CTimedCB& operator=(const CTimedCB&) = delete; + CTimedCB(CTimedCB&& rhs) = delete; + CTimedCB& operator=(CTimedCB&& rhs) = delete; + /** * @brief Start the timer. * @@ -97,7 +102,10 @@ namespace eCAL { if (!m_running) return(false); m_stop = true; - m_thread.join(); + // Wait for the callback thread to finish + if (m_thread.joinable()) { + m_thread.join(); + } m_running = false; return(true); } diff --git a/ecal/core/src/io/shm/ecal_memfile_pool.cpp b/ecal/core/src/io/shm/ecal_memfile_pool.cpp index 04ac67929a..12afb7fdb0 100644 --- a/ecal/core/src/io/shm/ecal_memfile_pool.cpp +++ b/ecal/core/src/io/shm/ecal_memfile_pool.cpp @@ -309,7 +309,10 @@ namespace eCAL { } - CMemFileThreadPool::~CMemFileThreadPool() = default; + CMemFileThreadPool::~CMemFileThreadPool() + { + Destroy(); + } void CMemFileThreadPool::Create() { diff --git a/ecal/core/src/io/shm/ecal_memfile_pool.h b/ecal/core/src/io/shm/ecal_memfile_pool.h index 7357055eaf..f0b0640488 100644 --- a/ecal/core/src/io/shm/ecal_memfile_pool.h +++ b/ecal/core/src/io/shm/ecal_memfile_pool.h @@ -50,6 +50,11 @@ namespace eCAL CMemFileObserver(); ~CMemFileObserver(); + CMemFileObserver(const CMemFileObserver&) = delete; + CMemFileObserver& operator=(const CMemFileObserver&) = delete; + CMemFileObserver(CMemFileObserver&& rhs) = delete; + CMemFileObserver& operator=(CMemFileObserver&& rhs) = delete; + bool Create(const std::string& memfile_name_, const std::string& memfile_event_); bool Destroy(); diff --git a/ecal/core/src/time/ecal_timer.cpp b/ecal/core/src/time/ecal_timer.cpp index 3422ec2bdc..083e94463e 100644 --- a/ecal/core/src/time/ecal_timer.cpp +++ b/ecal/core/src/time/ecal_timer.cpp @@ -38,6 +38,10 @@ namespace eCAL CTimerImpl(const int timeout_, TimerCallbackT callback_, const int delay_) : m_stop(false), m_running(false) { Start(timeout_, callback_, delay_); } virtual ~CTimerImpl() { Stop(); } + CTimerImpl(const CTimerImpl&) = delete; + CTimerImpl& operator=(const CTimerImpl&) = delete; + CTimerImpl(CTimerImpl&& rhs) = delete; + CTimerImpl& operator=(CTimerImpl&& rhs) = delete; bool Start(const int timeout_, TimerCallbackT callback_, const int delay_) { diff --git a/ecal/core/src/util/ecal_thread.h b/ecal/core/src/util/ecal_thread.h index c6493c3e76..49aec75437 100644 --- a/ecal/core/src/util/ecal_thread.h +++ b/ecal/core/src/util/ecal_thread.h @@ -44,6 +44,16 @@ namespace eCAL CCallbackThread(std::function callback) : callback_(callback) {} + ~CCallbackThread() + { + stop(); + } + + CCallbackThread(const CCallbackThread&) = delete; + CCallbackThread& operator=(const CCallbackThread&) = delete; + CCallbackThread(CCallbackThread&& rhs) = delete; + CCallbackThread& operator=(CCallbackThread&& rhs) = delete; + /** * @brief Start the callback thread with a specified timeout. * @param timeout The timeout duration for waiting in the callback thread. From c9eba531a09b1e18337b01b26bca22bf0132fd84 Mon Sep 17 00:00:00 2001 From: Kerstin Keller Date: Wed, 24 Jan 2024 14:09:43 +0100 Subject: [PATCH 2/2] Remove private copy member functions, because they are now deleted. --- ecal/core/include/ecal/ecal_timed_cb.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ecal/core/include/ecal/ecal_timed_cb.h b/ecal/core/include/ecal/ecal_timed_cb.h index fa4af6505b..82b95736a9 100644 --- a/ecal/core/include/ecal/ecal_timed_cb.h +++ b/ecal/core/include/ecal/ecal_timed_cb.h @@ -111,10 +111,6 @@ namespace eCAL } private: - // this object must not be copied. - CTimedCB(const CTimedCB&); - CTimedCB& operator=(const CTimedCB&); - void Thread(TimerCallbackT callback_, int timeout_, int delay_) { assert(callback_ != nullptr);