From 11ef1d10b8d1cce9fa6ba82e3c241876cb7f8fbc Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Wed, 1 Mar 2017 14:37:55 -0600 Subject: [PATCH 1/3] rtos: Return an error when a Thread is re-used Calling Thread::start multiple times leads to undefined behavior since the Thread class was not designed to handle being restarted. Return an error code if Thread::start is called a second time to prevent this behavior. --- rtos/Thread.cpp | 5 ++++- rtos/Thread.h | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/rtos/Thread.cpp b/rtos/Thread.cpp index 61bfe964c6..c168afde2f 100644 --- a/rtos/Thread.cpp +++ b/rtos/Thread.cpp @@ -44,6 +44,7 @@ namespace rtos { void Thread::constructor(osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { _tid = 0; + _finished = false; _dynamic_stack = (stack_pointer == NULL); #if defined(__MBED_CMSIS_RTOS_CA9) || defined(__MBED_CMSIS_RTOS_CM) @@ -74,7 +75,7 @@ void Thread::constructor(Callback task, osStatus Thread::start(Callback task) { _mutex.lock(); - if (_tid != 0) { + if ((_tid != 0) || _finished) { _mutex.unlock(); return osErrorParameter; } @@ -117,6 +118,7 @@ osStatus Thread::terminate() { osThreadId local_id = _tid; _join_sem.release(); _tid = (osThreadId)NULL; + _finished = true; ret = osThreadTerminate(local_id); @@ -367,6 +369,7 @@ void Thread::_thunk(const void * thread_ptr) t->_task(); t->_mutex.lock(); t->_tid = (osThreadId)NULL; + t->_finished = true; t->_join_sem.release(); // rtos will release the mutex automatically } diff --git a/rtos/Thread.h b/rtos/Thread.h index 6c480d74b7..1df34c3712 100644 --- a/rtos/Thread.h +++ b/rtos/Thread.h @@ -197,6 +197,7 @@ public: /** Starts a thread executing the specified function. @param task function to be executed by this thread. @return status code that indicates the execution status of the function. + @note a thread can only be started once */ osStatus start(mbed::Callback task); @@ -344,9 +345,10 @@ private: mbed::Callback _task; osThreadId _tid; osThreadDef_t _thread_def; - bool _dynamic_stack; Semaphore _join_sem; Mutex _mutex; + bool _dynamic_stack; + bool _finished; }; } From 05403d4231a5665eac69f17bf9c0ce87c323239e Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Fri, 3 Mar 2017 11:31:50 -0600 Subject: [PATCH 2/3] rtos: Add Inactive return to thread get state If a thread hasn't been started return Inactive as the status when Thread::get_state() is called. --- rtos/Thread.cpp | 6 +++++- rtos/Thread.h | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/rtos/Thread.cpp b/rtos/Thread.cpp index c168afde2f..9969e09670 100644 --- a/rtos/Thread.cpp +++ b/rtos/Thread.cpp @@ -179,11 +179,15 @@ int32_t Thread::signal_clr(int32_t signals) { Thread::State Thread::get_state() { #if !defined(__MBED_CMSIS_RTOS_CA9) && !defined(__MBED_CMSIS_RTOS_CM) #ifdef CMSIS_OS_RTX - State status = Deleted; + State status; _mutex.lock(); if (_tid != NULL) { status = (State)_thread_def.tcb.state; + } else if (_finished) { + status = Deleted; + } else { + status = Inactive; } _mutex.unlock(); diff --git a/rtos/Thread.h b/rtos/Thread.h index 1df34c3712..edad0840e9 100644 --- a/rtos/Thread.h +++ b/rtos/Thread.h @@ -252,7 +252,7 @@ public: /** State of the Thread */ enum State { - Inactive, /**< Not created or terminated */ + Inactive, /**< Not created */ Ready, /**< Ready to run */ Running, /**< Running */ WaitingDelay, /**< Waiting for a delay to occur */ From ab4da40e81c8d08deba5ac10e37a888afcbb05f1 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Mon, 6 Mar 2017 11:54:17 -0600 Subject: [PATCH 3/3] rtos: Prevent Thread class from being copied Make the copy constructor and assignment operator private to prevent them from being used. --- rtos/Thread.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rtos/Thread.h b/rtos/Thread.h index edad0840e9..d2bae4fb01 100644 --- a/rtos/Thread.h +++ b/rtos/Thread.h @@ -331,6 +331,10 @@ public: virtual ~Thread(); private: + /* disallow copy constructor and assignment operators */ + Thread(const Thread&); + Thread& operator=(const Thread&); + // Required to share definitions without // delegated constructors void constructor(osPriority priority=osPriorityNormal,