From 8b8606b98d4c0168a28f03fcbe33dac503328f16 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 22 Apr 2016 01:03:53 -0500 Subject: [PATCH 1/6] Add start function for separating object allocation from thread initialization Allows threads to started separately from when they are declared, avoiding the need to dynamically allocate threads at runtime. --- core/mbed-rtos/rtos/Thread.cpp | 19 +++++++++++++++++++ core/mbed-rtos/rtos/Thread.h | 17 +++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/core/mbed-rtos/rtos/Thread.cpp b/core/mbed-rtos/rtos/Thread.cpp index ed8270a36a..5ebb72d005 100644 --- a/core/mbed-rtos/rtos/Thread.cpp +++ b/core/mbed-rtos/rtos/Thread.cpp @@ -26,8 +26,22 @@ namespace rtos { +Thread::Thread() { + _tid = NULL; +} + Thread::Thread(void (*task)(void const *argument), void *argument, osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { + _tid = NULL; + start(task, argument, priority, stack_size, stack_pointer); +} + +osStatus Thread::start(void (*task)(void const *argument), void *argument, + osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { + if (_tid != NULL) { + return osErrorResource; + } + #ifdef __MBED_CMSIS_RTOS_CM _thread_def.pthread = task; _thread_def.tpriority = priority; @@ -48,6 +62,11 @@ Thread::Thread(void (*task)(void const *argument), void *argument, } #endif _tid = osThreadCreate(&_thread_def, argument); + if (_tid == NULL) { + return osErrorResource; + } + + return osOK; } osStatus Thread::terminate() { diff --git a/core/mbed-rtos/rtos/Thread.h b/core/mbed-rtos/rtos/Thread.h index 3e787983ab..0da6764a00 100644 --- a/core/mbed-rtos/rtos/Thread.h +++ b/core/mbed-rtos/rtos/Thread.h @@ -30,6 +30,10 @@ namespace rtos { /** The Thread class allow defining, creating, and controlling thread functions in the system. */ class Thread { public: + /** Allocate a new thread without starting execution + */ + Thread(); + /** Create a new thread, and start it executing the specified function. @param task function to be executed by this thread. @param argument pointer that is passed to the thread function as start argument. (default: NULL). @@ -42,6 +46,19 @@ public: uint32_t stack_size=DEFAULT_STACK_SIZE, unsigned char *stack_pointer=NULL); + /** Starts a thread executing the specified function. + @param task function to be executed by this thread. + @param argument pointer that is passed to the thread function as start argument. (default: NULL). + @param priority initial priority of the thread function. (default: osPriorityNormal). + @param stack_size stack size (in bytes) requirements for the thread function. (default: DEFAULT_STACK_SIZE). + @param stack_pointer pointer to the stack area to be used by this thread (default: NULL). + @return status code that indicates the execution status of the function. + */ + osStatus start(void (*task)(void const *argument), void *argument=NULL, + osPriority priority=osPriorityNormal, + uint32_t stack_size=DEFAULT_STACK_SIZE, + unsigned char *stack_pointer=NULL); + /** Terminate execution of a thread and remove it from Active Threads @return status code that indicates the execution status of the function. */ From d15cd7826ade05f28499c84fb4f5c5ae2da9fcc4 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 22 Apr 2016 01:12:40 -0500 Subject: [PATCH 2/6] Add Thread::join call for exiting threads without forceful termination Allows multiple threads to join without forceful termination. Allows threads to synchronize on cleanup. --- core/mbed-rtos/rtos/Thread.cpp | 16 +++++++++++++++- core/mbed-rtos/rtos/Thread.h | 5 +++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/core/mbed-rtos/rtos/Thread.cpp b/core/mbed-rtos/rtos/Thread.cpp index 5ebb72d005..82ce688119 100644 --- a/core/mbed-rtos/rtos/Thread.cpp +++ b/core/mbed-rtos/rtos/Thread.cpp @@ -39,7 +39,7 @@ Thread::Thread(void (*task)(void const *argument), void *argument, osStatus Thread::start(void (*task)(void const *argument), void *argument, osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { if (_tid != NULL) { - return osErrorResource; + return osErrorParameter; } #ifdef __MBED_CMSIS_RTOS_CM @@ -73,6 +73,20 @@ osStatus Thread::terminate() { return osThreadTerminate(_tid); } +osStatus Thread::join() { + while (true) { + uint8_t state = get_state(); + if (state == Thread::Inactive || state == osErrorParameter) { + return osOK; + } + + osStatus status = yield(); + if (status != osOK) { + return status; + } + } +} + osStatus Thread::set_priority(osPriority priority) { return osThreadSetPriority(_tid, priority); } diff --git a/core/mbed-rtos/rtos/Thread.h b/core/mbed-rtos/rtos/Thread.h index 0da6764a00..c172f88930 100644 --- a/core/mbed-rtos/rtos/Thread.h +++ b/core/mbed-rtos/rtos/Thread.h @@ -59,6 +59,11 @@ public: uint32_t stack_size=DEFAULT_STACK_SIZE, unsigned char *stack_pointer=NULL); + /** Wait for thread to terminate + @return status code that indicates the execution status of the function. + */ + osStatus join(); + /** Terminate execution of a thread and remove it from Active Threads @return status code that indicates the execution status of the function. */ From 21e68f3c9116c4fa4e62bfb9eb88247845a70fc7 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 23 Apr 2016 03:40:05 -0500 Subject: [PATCH 3/6] Move configuration arguments to only constructor --- core/mbed-rtos/rtos/Thread.cpp | 30 +++++++++++++++++------------- core/mbed-rtos/rtos/Thread.h | 20 +++++++++++--------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/core/mbed-rtos/rtos/Thread.cpp b/core/mbed-rtos/rtos/Thread.cpp index 82ce688119..70ffa9e30c 100644 --- a/core/mbed-rtos/rtos/Thread.cpp +++ b/core/mbed-rtos/rtos/Thread.cpp @@ -26,38 +26,42 @@ namespace rtos { -Thread::Thread() { +Thread::Thread(osPriority priority, + uint32_t stack_size, unsigned char *stack_pointer) { _tid = NULL; + _priority = priority; + _stack_size = stack_size; + _stack_pointer = stack_pointer; } Thread::Thread(void (*task)(void const *argument), void *argument, osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { _tid = NULL; - start(task, argument, priority, stack_size, stack_pointer); + _priority = priority; + _stack_size = stack_size; + _stack_pointer = stack_pointer; + start(task, argument); } -osStatus Thread::start(void (*task)(void const *argument), void *argument, - osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { +osStatus Thread::start(void (*task)(void const *argument), void *argument) { if (_tid != NULL) { return osErrorParameter; } #ifdef __MBED_CMSIS_RTOS_CM _thread_def.pthread = task; - _thread_def.tpriority = priority; - _thread_def.stacksize = stack_size; - if (stack_pointer != NULL) { - _thread_def.stack_pointer = (uint32_t*)stack_pointer; - _dynamic_stack = false; + _thread_def.tpriority = _priority; + _thread_def.stacksize = _stack_size; + if (_stack_pointer != NULL) { + _thread_def.stack_pointer = (uint32_t*)_stack_pointer; } else { - _thread_def.stack_pointer = new uint32_t[stack_size/sizeof(uint32_t)]; + _thread_def.stack_pointer = new uint32_t[_stack_size/sizeof(uint32_t)]; if (_thread_def.stack_pointer == NULL) error("Error allocating the stack memory\n"); - _dynamic_stack = true; } //Fill the stack with a magic word for maximum usage checking - for (uint32_t i = 0; i < (stack_size / sizeof(uint32_t)); i++) { + for (uint32_t i = 0; i < (_stack_size / sizeof(uint32_t)); i++) { _thread_def.stack_pointer[i] = 0xE25A2EA5; } #endif @@ -191,7 +195,7 @@ void Thread::attach_idle_hook(void (*fptr)(void)) { Thread::~Thread() { terminate(); #ifdef __MBED_CMSIS_RTOS_CM - if (_dynamic_stack) { + if (_stack_pointer == NULL) { delete[] (_thread_def.stack_pointer); } #endif diff --git a/core/mbed-rtos/rtos/Thread.h b/core/mbed-rtos/rtos/Thread.h index c172f88930..1cbc93afa4 100644 --- a/core/mbed-rtos/rtos/Thread.h +++ b/core/mbed-rtos/rtos/Thread.h @@ -31,8 +31,13 @@ namespace rtos { class Thread { public: /** Allocate a new thread without starting execution + @param priority initial priority of the thread function. (default: osPriorityNormal). + @param stack_size stack size (in bytes) requirements for the thread function. (default: DEFAULT_STACK_SIZE). + @param stack_pointer pointer to the stack area to be used by this thread (default: NULL). */ - Thread(); + Thread(osPriority priority=osPriorityNormal, + uint32_t stack_size=DEFAULT_STACK_SIZE, + unsigned char *stack_pointer=NULL); /** Create a new thread, and start it executing the specified function. @param task function to be executed by this thread. @@ -49,15 +54,9 @@ public: /** Starts a thread executing the specified function. @param task function to be executed by this thread. @param argument pointer that is passed to the thread function as start argument. (default: NULL). - @param priority initial priority of the thread function. (default: osPriorityNormal). - @param stack_size stack size (in bytes) requirements for the thread function. (default: DEFAULT_STACK_SIZE). - @param stack_pointer pointer to the stack area to be used by this thread (default: NULL). @return status code that indicates the execution status of the function. */ - osStatus start(void (*task)(void const *argument), void *argument=NULL, - osPriority priority=osPriorityNormal, - uint32_t stack_size=DEFAULT_STACK_SIZE, - unsigned char *stack_pointer=NULL); + osStatus start(void (*task)(void const *argument), void *argument=NULL); /** Wait for thread to terminate @return status code that indicates the execution status of the function. @@ -164,7 +163,10 @@ public: private: osThreadId _tid; osThreadDef_t _thread_def; - bool _dynamic_stack; + + osPriority _priority; + uint32_t _stack_size; + unsigned char *_stack_pointer; }; } From da6571cc249a09ad4968cb78c344c0c933e7912b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 23 Apr 2016 12:10:47 -0500 Subject: [PATCH 4/6] Add documentation on invalid thread functions in irq --- core/mbed-rtos/rtos/Thread.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/mbed-rtos/rtos/Thread.h b/core/mbed-rtos/rtos/Thread.h index 1cbc93afa4..d9a5cbe68d 100644 --- a/core/mbed-rtos/rtos/Thread.h +++ b/core/mbed-rtos/rtos/Thread.h @@ -60,6 +60,7 @@ public: /** Wait for thread to terminate @return status code that indicates the execution status of the function. + @note not callable from interrupt */ osStatus join(); @@ -134,17 +135,20 @@ public: @param signals wait until all specified signal flags set or 0 for any single signal flag. @param millisec timeout value or 0 in case of no time-out. (default: osWaitForever). @return event flag information or error code. + @note not callable from interrupt */ static osEvent signal_wait(int32_t signals, uint32_t millisec=osWaitForever); /** Wait for a specified time period in millisec: @param millisec time delay value @return status code that indicates the execution status of the function. + @note not callable from interrupt */ static osStatus wait(uint32_t millisec); /** Pass control to next thread that is in state READY. @return status code that indicates the execution status of the function. + @note not callable from interrupt */ static osStatus yield(); From ef291e79d969d896971922c49a5f79e66c476766 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 23 Apr 2016 15:32:50 -0500 Subject: [PATCH 5/6] Add unit tests over spawning/joining threads with thread functions --- .../TESTS/mbed-rtos/threads/main.cpp | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 core/mbed-rtos/TESTS/mbed-rtos/threads/main.cpp diff --git a/core/mbed-rtos/TESTS/mbed-rtos/threads/main.cpp b/core/mbed-rtos/TESTS/mbed-rtos/threads/main.cpp new file mode 100644 index 0000000000..030beec2da --- /dev/null +++ b/core/mbed-rtos/TESTS/mbed-rtos/threads/main.cpp @@ -0,0 +1,110 @@ +#include "mbed.h" +#include "test_env.h" +#include "unity.h" +#include "utest.h" +#include "rtos.h" + + +using namespace utest::v1; + + +// Tasks with different functions to test on threads +void increment(const void *var) { + (*(int *)var)++; +} + +void increment_with_yield(const void *var) { + Thread::yield(); + (*(int *)var)++; +} + +void increment_with_wait(const void *var) { + Thread::wait(100); + (*(int *)var)++; +} + +void increment_with_child(const void *var) { + Thread child(increment, (void*)var); + child.join(); +} + +void increment_with_murder(const void *var) { + Thread child(increment_with_wait, (void*)var); + // Kill child before it can increment var + child.terminate(); + (*(int *)var)++; +} + + +// Tests that spawn tasks in different configurations +template +void test_single_thread() { + int var = 0; + Thread thread(F, &var); + thread.join(); + TEST_ASSERT_EQUAL(var, 1); +} + +template +void test_parallel_threads() { + int var = 0; + Thread *threads[N]; + + for (int i = 0; i < N; i++) { + threads[i] = new Thread(F, &var); + } + + for (int i = 0; i < N; i++) { + threads[i]->join(); + delete threads[i]; + } + + TEST_ASSERT_EQUAL(var, N); +} + +template +void test_serial_threads() { + int var = 0; + + for (int i = 0; i < N; i++) { + Thread thread(F, &var); + thread.join(); + } + + TEST_ASSERT_EQUAL(var, N); +} + + +status_t test_setup(const size_t number_of_cases) { + GREENTEA_SETUP(40, "default_auto"); + return verbose_test_setup_handler(number_of_cases); +} + +// Test cases +Case cases[] = { + Case("Testing single thread", test_single_thread), + Case("Testing parallel threads", test_parallel_threads<3, increment>), + Case("Testing serial threads", test_serial_threads<10, increment>), + + Case("Testing single thread with yield", test_single_thread), + Case("Testing parallel threads with yield", test_parallel_threads<3, increment_with_yield>), + Case("Testing serial threads with yield", test_serial_threads<10, increment_with_yield>), + + Case("Testing single thread with wait", test_single_thread), + Case("Testing parallel threads with wait", test_parallel_threads<3, increment_with_wait>), + Case("Testing serial threads with wait", test_serial_threads<10, increment_with_wait>), + + Case("Testing single thread with child", test_single_thread), + Case("Testing parallel threads with child", test_parallel_threads<3, increment_with_child>), + Case("Testing serial threads with child", test_serial_threads<10, increment_with_child>), + + Case("Testing single thread with murder", test_single_thread), + Case("Testing parallel threads with murder", test_parallel_threads<3, increment_with_murder>), + Case("Testing serial threads with murder", test_serial_threads<10, increment_with_murder>), +}; + +Specification specification(test_setup, cases); + +int main() { + return !Harness::run(specification); +} From 8a37762866a1820c0a5333943b088056bbd2a063 Mon Sep 17 00:00:00 2001 From: Niklas Hauser Date: Tue, 10 May 2016 14:02:58 +0100 Subject: [PATCH 6/6] Smaller Thread class with clearer error messages. --- core/mbed-rtos/rtos/Thread.cpp | 55 ++++++++++++++++++++-------------- core/mbed-rtos/rtos/Thread.h | 5 +--- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/core/mbed-rtos/rtos/Thread.cpp b/core/mbed-rtos/rtos/Thread.cpp index 70ffa9e30c..402295e8d7 100644 --- a/core/mbed-rtos/rtos/Thread.cpp +++ b/core/mbed-rtos/rtos/Thread.cpp @@ -27,20 +27,35 @@ namespace rtos { Thread::Thread(osPriority priority, - uint32_t stack_size, unsigned char *stack_pointer) { - _tid = NULL; - _priority = priority; - _stack_size = stack_size; - _stack_pointer = stack_pointer; + uint32_t stack_size, unsigned char *stack_pointer): + _tid(NULL), _dynamic_stack(stack_pointer == NULL) { +#ifdef __MBED_CMSIS_RTOS_CM + _thread_def.tpriority = priority; + _thread_def.stacksize = stack_size; + _thread_def.stack_pointer = (uint32_t*)stack_pointer; +#endif } Thread::Thread(void (*task)(void const *argument), void *argument, - osPriority priority, uint32_t stack_size, unsigned char *stack_pointer) { - _tid = NULL; - _priority = priority; - _stack_size = stack_size; - _stack_pointer = stack_pointer; - start(task, argument); + osPriority priority, uint32_t stack_size, unsigned char *stack_pointer): + _tid(NULL), _dynamic_stack(stack_pointer == NULL) { +#ifdef __MBED_CMSIS_RTOS_CM + _thread_def.tpriority = priority; + _thread_def.stacksize = stack_size; + _thread_def.stack_pointer = (uint32_t*)stack_pointer; +#endif + switch(start(task, argument)) { + case osErrorResource: + error("OS ran out of threads!\n"); + break; + case osErrorParameter: + error("Thread already running!\n"); + break; + case osErrorNoMemory: + error("Error allocating the stack memory\n"); + default: + break; + } } osStatus Thread::start(void (*task)(void const *argument), void *argument) { @@ -50,26 +65,22 @@ osStatus Thread::start(void (*task)(void const *argument), void *argument) { #ifdef __MBED_CMSIS_RTOS_CM _thread_def.pthread = task; - _thread_def.tpriority = _priority; - _thread_def.stacksize = _stack_size; - if (_stack_pointer != NULL) { - _thread_def.stack_pointer = (uint32_t*)_stack_pointer; - } else { - _thread_def.stack_pointer = new uint32_t[_stack_size/sizeof(uint32_t)]; + if (_thread_def.stack_pointer == NULL) { + _thread_def.stack_pointer = new uint32_t[_thread_def.stacksize/sizeof(uint32_t)]; if (_thread_def.stack_pointer == NULL) - error("Error allocating the stack memory\n"); + return osErrorNoMemory; } - + //Fill the stack with a magic word for maximum usage checking - for (uint32_t i = 0; i < (_stack_size / sizeof(uint32_t)); i++) { + for (uint32_t i = 0; i < (_thread_def.stacksize / sizeof(uint32_t)); i++) { _thread_def.stack_pointer[i] = 0xE25A2EA5; } #endif _tid = osThreadCreate(&_thread_def, argument); if (_tid == NULL) { + if (_dynamic_stack) delete[] (_thread_def.stack_pointer); return osErrorResource; } - return osOK; } @@ -195,7 +206,7 @@ void Thread::attach_idle_hook(void (*fptr)(void)) { Thread::~Thread() { terminate(); #ifdef __MBED_CMSIS_RTOS_CM - if (_stack_pointer == NULL) { + if (_dynamic_stack) { delete[] (_thread_def.stack_pointer); } #endif diff --git a/core/mbed-rtos/rtos/Thread.h b/core/mbed-rtos/rtos/Thread.h index d9a5cbe68d..275f6fe5c9 100644 --- a/core/mbed-rtos/rtos/Thread.h +++ b/core/mbed-rtos/rtos/Thread.h @@ -167,10 +167,7 @@ public: private: osThreadId _tid; osThreadDef_t _thread_def; - - osPriority _priority; - uint32_t _stack_size; - unsigned char *_stack_pointer; + bool _dynamic_stack; }; }