From a6dea64106370fb0574233b48b7bda7c2c51a1d7 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 23 Apr 2021 10:01:37 +1000 Subject: [PATCH] pthread: Add support for PTHREAD_COND_INITIALIZER Includes unit test for condition variables in C (previous test was C++ only) --- components/pthread/pthread.c | 10 +- components/pthread/pthread_cond_var.c | 39 +++++- components/pthread/pthread_internal.h | 2 + .../pthread/test/test_pthread_cond_var.c | 115 ++++++++++++++++++ docs/en/api-reference/system/pthread.rst | 3 +- 5 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 components/pthread/test/test_pthread_cond_var.c diff --git a/components/pthread/pthread.c b/components/pthread/pthread.c index c31ed9f68a..02178f56e5 100644 --- a/components/pthread/pthread.c +++ b/components/pthread/pthread.c @@ -66,7 +66,7 @@ typedef struct { static SemaphoreHandle_t s_threads_mux = NULL; -static portMUX_TYPE s_mutex_init_lock = portMUX_INITIALIZER_UNLOCKED; +portMUX_TYPE pthread_lazy_init_lock = portMUX_INITIALIZER_UNLOCKED; // Used for mutexes and cond vars static SLIST_HEAD(esp_thread_list_head, esp_pthread_entry) s_threads_list = SLIST_HEAD_INITIALIZER(s_threads_list); static pthread_key_t s_pthread_cfg_key; @@ -581,6 +581,10 @@ int pthread_mutex_destroy(pthread_mutex_t *mutex) if (!mutex) { return EINVAL; } + if ((intptr_t) *mutex == PTHREAD_MUTEX_INITIALIZER) { + return 0; // Static mutex was never initialized + } + mux = (esp_pthread_mutex_t *)*mutex; if (!mux) { return EINVAL; @@ -634,11 +638,11 @@ static int pthread_mutex_init_if_static(pthread_mutex_t *mutex) { int res = 0; if ((intptr_t) *mutex == PTHREAD_MUTEX_INITIALIZER) { - portENTER_CRITICAL(&s_mutex_init_lock); + portENTER_CRITICAL(&pthread_lazy_init_lock); if ((intptr_t) *mutex == PTHREAD_MUTEX_INITIALIZER) { res = pthread_mutex_init(mutex, NULL); } - portEXIT_CRITICAL(&s_mutex_init_lock); + portEXIT_CRITICAL(&pthread_lazy_init_lock); } return res; } diff --git a/components/pthread/pthread_cond_var.c b/components/pthread/pthread_cond_var.c index 7ddb8530c2..78265f85d9 100644 --- a/components/pthread/pthread_cond_var.c +++ b/components/pthread/pthread_cond_var.c @@ -26,6 +26,7 @@ #include "freertos/task.h" #include "freertos/semphr.h" #include "freertos/list.h" +#include "pthread_internal.h" #include #include @@ -43,12 +44,32 @@ typedef struct esp_pthread_cond { TAILQ_HEAD(, esp_pthread_cond_waiter) waiter_list; ///< head of the list of semaphores } esp_pthread_cond_t; -int pthread_cond_signal(pthread_cond_t *cv) +static int s_check_and_init_if_static(pthread_cond_t *cv) { + int res = 0; + if (cv == NULL || *cv == (pthread_cond_t) 0) { return EINVAL; } + if (*cv == PTHREAD_COND_INITIALIZER) { + portENTER_CRITICAL(&pthread_lazy_init_lock); + if (*cv == PTHREAD_COND_INITIALIZER) { + res = pthread_cond_init(cv, NULL); + } + portEXIT_CRITICAL(&pthread_lazy_init_lock); + } + + return res; +} + +int pthread_cond_signal(pthread_cond_t *cv) +{ + int res = s_check_and_init_if_static(cv); + if (res) { + return res; + } + esp_pthread_cond_t *cond = (esp_pthread_cond_t *) *cv; _lock_acquire_recursive(&cond->lock); @@ -64,8 +85,9 @@ int pthread_cond_signal(pthread_cond_t *cv) int pthread_cond_broadcast(pthread_cond_t *cv) { - if (cv == NULL || *cv == (pthread_cond_t) 0) { - return EINVAL; + int res = s_check_and_init_if_static(cv); + if (res) { + return res; } esp_pthread_cond_t *cond = (esp_pthread_cond_t *) *cv; @@ -90,8 +112,9 @@ int pthread_cond_timedwait(pthread_cond_t *cv, pthread_mutex_t *mut, const struc int ret; TickType_t timeout_ticks; - if (cv == NULL || *cv == (pthread_cond_t) 0) { - return EINVAL; + int res = s_check_and_init_if_static(cv); + if (res) { + return res; } esp_pthread_cond_t *cond = (esp_pthread_cond_t *) *cv; @@ -180,8 +203,14 @@ int pthread_cond_destroy(pthread_cond_t *cv) if (cv == NULL || *cv == (pthread_cond_t) 0) { return EINVAL; } + if (*cv == PTHREAD_COND_INITIALIZER) { + return 0; // never initialized + } esp_pthread_cond_t *cond = (esp_pthread_cond_t *) *cv; + if (!cond) { + return EINVAL; + } _lock_acquire_recursive(&cond->lock); if (!TAILQ_EMPTY(&cond->waiter_list)) { diff --git a/components/pthread/pthread_internal.h b/components/pthread/pthread_internal.h index a4e4cfd55d..80ddb89395 100644 --- a/components/pthread/pthread_internal.h +++ b/components/pthread/pthread_internal.h @@ -14,3 +14,5 @@ #pragma once void pthread_internal_local_storage_destructor_callback(void); + +extern portMUX_TYPE pthread_lazy_init_lock; diff --git a/components/pthread/test/test_pthread_cond_var.c b/components/pthread/test/test_pthread_cond_var.c new file mode 100644 index 0000000000..6cd18f5a61 --- /dev/null +++ b/components/pthread/test/test_pthread_cond_var.c @@ -0,0 +1,115 @@ +#include +#include "unity.h" + +typedef struct { + pthread_cond_t *cond; + pthread_mutex_t *mutex; + unsigned delay_ms; +} thread_args_t; + +static void *thread_signals(void *arg) +{ + const thread_args_t *targs = (thread_args_t *)arg; + int r; + + r = pthread_mutex_lock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_cond_signal(targs->cond); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_mutex_unlock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + usleep(targs->delay_ms * 1000); + + r = pthread_mutex_lock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_cond_broadcast(targs->cond); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_mutex_unlock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + return NULL; +} + +static void *thread_waits(void *arg) +{ + const thread_args_t *targs = (thread_args_t *)arg; + int r; + + r = pthread_mutex_lock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_cond_wait(targs->cond, targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_mutex_unlock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + usleep(targs->delay_ms * 1000); + + r = pthread_mutex_lock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + struct timespec two_seconds; + clock_gettime(CLOCK_REALTIME, &two_seconds); + two_seconds.tv_sec += 2; + r = pthread_cond_timedwait(targs->cond, targs->mutex, &two_seconds); + TEST_ASSERT_EQUAL_INT(0, r); + + r = pthread_mutex_unlock(targs->mutex); + TEST_ASSERT_EQUAL_INT(0, r); + + return NULL; +} + +#define NUM_THREADS 3 + +TEST_CASE("pthread cond wait", "[pthread]") +{ + int r; + pthread_cond_t cond = PTHREAD_COND_INITIALIZER; + pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; + + struct { + thread_args_t args; + pthread_t thread; + } wait[NUM_THREADS]; + struct { + thread_args_t args; + pthread_t thread; + } signal[NUM_THREADS]; + + wait[0].args.delay_ms = 50; + wait[1].args.delay_ms = 100; + wait[2].args.delay_ms = 200; + + signal[0].args.delay_ms = 30; + signal[1].args.delay_ms = 150; + signal[2].args.delay_ms = 500; // highest delay, ensure that broadcast will be received by all waiter threads + + for (int i = 0; i < NUM_THREADS; i++) { + wait[i].args.cond = &cond; + wait[i].args.mutex = &mutex; + signal[i].args.cond = &cond; + signal[i].args.mutex = &mutex; + + r = pthread_create(&signal[i].thread, NULL, thread_signals, &signal[i].args); + TEST_ASSERT_EQUAL_INT(0, r); + r = pthread_create(&wait[i].thread, NULL, thread_waits, &wait[i].args); + TEST_ASSERT_EQUAL_INT(0, r); + } + + for (int i = 0; i < NUM_THREADS; i++) { + r = pthread_join(signal[i].thread, NULL); + TEST_ASSERT_EQUAL_INT(0, r); + pthread_join(wait[i].thread, NULL); + TEST_ASSERT_EQUAL_INT(0, r); + } + + pthread_mutex_destroy(&cond); + pthread_mutex_destroy(&mutex); +} diff --git a/docs/en/api-reference/system/pthread.rst b/docs/en/api-reference/system/pthread.rst index 94aba961c7..c16ebf8a1e 100644 --- a/docs/en/api-reference/system/pthread.rst +++ b/docs/en/api-reference/system/pthread.rst @@ -92,6 +92,8 @@ Condition Variables * ``pthread_cond_wait()`` * ``pthread_cond_timedwait()`` +Static initializer constant ``PTHREAD_COND_INITIALIZER`` is supported. + .. note:: These functions can be called from tasks created using either pthread or FreeRTOS APIs Thread-Specific Data @@ -113,7 +115,6 @@ The ``pthread.h`` header is a standard header and includes additional APIs and f * ``pthread_cancel()`` returns ``ENOSYS`` if called. * ``pthread_condattr_init()`` returns ``ENOSYS`` if called. -* ``PTHREAD_COND_INITIALIZER`` static initializer constant is not implemented and will crash if passed to a function. Other POSIX Threads functions (not listed here) are not implemented and will produce either a compiler or a linker error if referenced from an ESP-IDF application. If you identify a useful API that you would like to see implemented in ESP-IDF, please open a `feature request on GitHub ` with the details.