From 5795ccc8061cda529741c41b33f2aa2c95bcb464 Mon Sep 17 00:00:00 2001 From: Alexey Gerenkov Date: Thu, 28 Sep 2017 21:18:29 +0300 Subject: [PATCH] freertos: Fixes deadlock in pthread_once for init_routines calling pthread_once --- components/pthread/pthread.c | 8 ++++--- components/pthread/test/test_pthread_cxx.cpp | 24 +++++++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/components/pthread/pthread.c b/components/pthread/pthread.c index c7d6f67075..7817355fbb 100644 --- a/components/pthread/pthread.c +++ b/components/pthread/pthread.c @@ -72,7 +72,7 @@ static int IRAM_ATTR pthread_mutex_lock_internal(esp_pthread_mutex_t *mux, TickT esp_err_t esp_pthread_init(void) { vListInitialise((List_t *)&s_threads_list); - s_once_mux = xSemaphoreCreateMutex(); + s_once_mux = xSemaphoreCreateRecursiveMutex(); if (s_once_mux == NULL) { return ESP_ERR_NO_MEM; } @@ -377,7 +377,9 @@ int pthread_once(pthread_once_t *once_control, void (*init_routine)(void)) TaskHandle_t cur_task = xTaskGetCurrentTaskHandle(); // do not take mutex if OS is not running yet if (xTaskGetSchedulerState() == taskSCHEDULER_NOT_STARTED || - !cur_task || xSemaphoreTake(s_once_mux, portMAX_DELAY) == pdTRUE) + // init_routine can call pthread_once for another objects, so use recursive mutex + // FIXME: behaviour is undefined if init_routine calls pthread_once for the same object in the current context + !cur_task || xSemaphoreTakeRecursive(s_once_mux, portMAX_DELAY) == pdTRUE) { if (!once_control->init_executed) { ESP_LOGV(TAG, "%s: call init_routine %p", __FUNCTION__, once_control); @@ -385,7 +387,7 @@ int pthread_once(pthread_once_t *once_control, void (*init_routine)(void)) once_control->init_executed = 1; } if (cur_task) { - xSemaphoreGive(s_once_mux); + xSemaphoreGiveRecursive(s_once_mux); } } else diff --git a/components/pthread/test/test_pthread_cxx.cpp b/components/pthread/test/test_pthread_cxx.cpp index bed906f610..ced961d09f 100644 --- a/components/pthread/test/test_pthread_cxx.cpp +++ b/components/pthread/test/test_pthread_cxx.cpp @@ -2,6 +2,8 @@ #include #include #include +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" #include "unity.h" #if __GTHREADS && __GTHREADS_CXX0X @@ -61,7 +63,7 @@ static void thread_main() } } -TEST_CASE("pthread CXX", "[pthread]") +TEST_CASE("pthread C++", "[pthread]") { global_sp.reset(new int(1)); @@ -85,16 +87,32 @@ TEST_CASE("pthread CXX", "[pthread]") } } -static void task_test_sandbox(void *ignore) { +static void task_test_sandbox(void *arg) +{ + bool *running = (bool *)arg; + ESP_LOGI(TAG, "About to create a string stream"); std::stringstream ss; ESP_LOGI(TAG, "About to write to string stream"); ss << "Hello World!"; + ESP_LOGI(TAG, "About to extract from stringstream"); + ESP_LOGI(TAG, "Text: %s", ss.str().c_str()); + + if (running) { + *running = false; + vTaskDelete(NULL); + } } -TEST_CASE("pthread CXX 2", "[pthread]") +TEST_CASE("pthread mix C/C++", "[pthread]") { + bool running = true; + std::thread t1(task_test_sandbox, (void *)NULL); + xTaskCreatePinnedToCore((TaskFunction_t)&task_test_sandbox, "task_test_sandbox", 2048, &running, 5, NULL, 0); + while (running) { + vTaskDelay(1); + } if (t1.joinable()) { std::cout << "Join thread " << std::hex << t1.get_id() << std::endl; t1.join();