recursive mutex implemented, some refactoring.

mattheweshleman · mattheweshleman · commit 78c33d399c86 · 2024-10-28T15:28:25.000-05:00
diff --git a/cpputest-for-freertos-lib/src/cpputest_for_freertos_fake_queue.hpp b/cpputest-for-freertos-lib/src/cpputest_for_freertos_fake_queue.hpp
@@ -12,6 +12,12 @@ typedef struct QueueDefinition
     UBaseType_t itemSize = {};
     uint8_t queueType = {};
     std::deque<std::vector<uint8_t>> queue = {};
+    uint64_t recursiveCallCount = {};
 } FakeQueue;
 
+namespace cms {
+    BaseType_t InternalQueueReceive(FakeQueue *queue, void * const buffer);
+    BaseType_t InternalQueueReceive(FakeQueue *queue);
+} //namespace cms
+
 #endif //CPPUTEST_FOR_FREERTOS_LIB_CPPUTEST_FOR_FREERTOS_FAKE_QUEUE_HPP
diff --git a/cpputest-for-freertos-lib/src/cpputest_for_freertos_mutex.cpp b/cpputest-for-freertos-lib/src/cpputest_for_freertos_mutex.cpp
@@ -84,27 +84,69 @@ namespace cms {
     } //namespace
 }//namespace
 
-extern "C" QueueHandle_t xQueueCreateMutex( const uint8_t ucQueueType )
+extern "C" QueueHandle_t xQueueCreateMutex(const uint8_t queueType)
 {
-    (void)ucQueueType;
-    auto mutex = xQueueGenericCreate(1, 0, ucQueueType);
-    switch (ucQueueType) {
+    (void)queueType;
+    auto mutex = xQueueGenericCreate(1, 0, queueType);
+    switch (queueType) {
         case queueQUEUE_TYPE_MUTEX:
             //wasn't documented, but in experiment, the standard mutex is created unlocked
             //(i.e) with one token available
             xSemaphoreGive(mutex);
             break;
         case queueQUEUE_TYPE_RECURSIVE_MUTEX:
-            //todo
+            xSemaphoreGive(mutex);
+            mutex->recursiveCallCount = 0;
             break;
         default:
             configASSERT(true == false);
-            break;
     }
     configASSERT(mutex != nullptr);
     if (cms::test::s_mutexes != nullptr)
     {
         cms::test::s_mutexes->push_back(mutex);
     }
     return mutex;
-}
+}
+
+extern "C" BaseType_t xQueueTakeMutexRecursive(QueueHandle_t mutex,
+                                               TickType_t ticks)
+{
+    (void) ticks; //not used
+
+    configASSERT(mutex != nullptr);
+    configASSERT(mutex->queueType == queueQUEUE_TYPE_RECURSIVE_MUTEX);
+
+    if (1 == uxSemaphoreGetCount(mutex))
+    {
+        auto rtn = cms::InternalQueueReceive(mutex);
+        if (rtn != pdTRUE)
+        {
+            return rtn;
+        }
+    }
+
+    mutex->recursiveCallCount++;
+    return pdTRUE;
+
+}
+
+extern "C" BaseType_t xQueueGiveMutexRecursive(QueueHandle_t mutex)
+{
+    configASSERT(mutex != nullptr);
+    configASSERT(mutex->queueType == queueQUEUE_TYPE_RECURSIVE_MUTEX);
+
+    if (0 == uxSemaphoreGetCount(mutex) && mutex->recursiveCallCount > 0)
+    {
+        mutex->recursiveCallCount--;
+        if (mutex->recursiveCallCount == 0)
+        {
+            return xSemaphoreGive(mutex);
+        }
+
+        return pdTRUE;
+    }
+
+    return pdFALSE;
+}
+
diff --git a/cpputest-for-freertos-lib/src/cpputest_for_freertos_queue.cpp b/cpputest-for-freertos-lib/src/cpputest_for_freertos_queue.cpp
@@ -67,10 +67,17 @@ extern "C" UBaseType_t uxQueueSpacesAvailable(const QueueHandle_t queue)
     return queue->queueLength - uxQueueMessagesWaiting(queue);
 }
 
-extern "C" BaseType_t xQueueReceive(QueueHandle_t queue, void * const buffer, TickType_t ticks)
+BaseType_t cms::InternalQueueReceive(FakeQueue * queue)
+{
+    configASSERT(queue != nullptr);
+
+    uint64_t dummy;
+    return cms::InternalQueueReceive(queue, &dummy);
+}
+
+BaseType_t cms::InternalQueueReceive(FakeQueue * queue, void * const buffer)
 {
     configASSERT(queue != nullptr);
-    (void)ticks; //in our unit testing fake, never honor ticks to wait.
 
     if (!queue->queue.empty())
     {
@@ -85,10 +92,19 @@ extern "C" BaseType_t xQueueReceive(QueueHandle_t queue, void * const buffer, Ti
     }
 }
 
+extern "C" BaseType_t xQueueReceive(QueueHandle_t queue, void * const buffer, TickType_t ticks)
+{
+    configASSERT(queue != nullptr);
+    configASSERT(buffer != nullptr);
+
+    (void)ticks; //in our unit testing fake, never honor ticks to wait.
+    return cms::InternalQueueReceive(queue, buffer);
+}
+
 extern "C" BaseType_t xQueueGenericSend(QueueHandle_t queue,
                                         const void * const itemToQueue,
                                         TickType_t ticks,
-                                        const BaseType_t copyPosition )
+                                        const BaseType_t copyPosition)
 {
     (void)ticks;
     configASSERT(queue != nullptr);
diff --git a/cpputest-for-freertos-lib/src/cpputest_for_freertos_semaphore.cpp b/cpputest-for-freertos-lib/src/cpputest_for_freertos_semaphore.cpp
@@ -33,15 +33,19 @@
 extern "C" BaseType_t xQueueSemaphoreTake(QueueHandle_t queue, TickType_t ticks)
 {
     (void) ticks;  //ignore in cpputest fake semaphore
+    configASSERT(queue != nullptr);
+    configASSERT(queue->queueType != queueQUEUE_TYPE_RECURSIVE_MUTEX);
 
-    uint64_t dummy;
-    return xQueueReceive(queue, &dummy, ticks);
+    return cms::InternalQueueReceive(queue);
 }
 
 
 extern "C" BaseType_t xQueueGiveFromISR(QueueHandle_t queue,
                                         BaseType_t * const pxHigherPriorityTaskWoken)
 {
+    configASSERT(queue != nullptr);
+    configASSERT(queue->queueType != queueQUEUE_TYPE_RECURSIVE_MUTEX);
+
     (void)pxHigherPriorityTaskWoken;
     return xSemaphoreGive(queue);
 }
diff --git a/cpputest-for-freertos-lib/tests/cpputest_for_freertos_mutex_tests.cpp b/cpputest-for-freertos-lib/tests/cpputest_for_freertos_mutex_tests.cpp
@@ -47,6 +47,13 @@ TEST_GROUP(MutexTests)
     void CreateMutex()
     {
         mMutexUnderTest = xSemaphoreCreateMutex();
+        CHECK_TRUE(nullptr != mMutexUnderTest);
+    }
+
+    void CreateRecursiveMutex()
+    {
+        mMutexUnderTest = xSemaphoreCreateRecursiveMutex();
+        CHECK_TRUE(nullptr != mMutexUnderTest);
     }
 };
 
@@ -87,6 +94,58 @@ TEST(MutexTests, library_does_not_detect_locked_mutex_that_was_deleted)
     xSemaphoreTake(mMutexUnderTest, 1000);
     CHECK_TRUE(cms::test::IsAnyMutexLocked());
 
+    vSemaphoreDelete(mMutexUnderTest);
+    mMutexUnderTest = nullptr;
+    CHECK_FALSE(cms::test::IsAnyMutexLocked());
+    cms::test::MutexTrackingTeardown();
+}
+
+TEST(MutexTests, can_create_a_recursive_mutex)
+{
+    CreateRecursiveMutex();
+    CHECK_EQUAL(1, uxSemaphoreGetCount(mMutexUnderTest));
+}
+
+TEST(MutexTests, can_lock_a_recursive_mutex_multiple_times)
+{
+    CreateRecursiveMutex();
+    CHECK_EQUAL(1, uxSemaphoreGetCount(mMutexUnderTest));
+    CHECK_EQUAL(pdTRUE, xSemaphoreTakeRecursive(mMutexUnderTest, 1000));
+    CHECK_EQUAL(pdTRUE, xSemaphoreTakeRecursive(mMutexUnderTest, 1000));
+    CHECK_EQUAL(pdTRUE, xSemaphoreTakeRecursive(mMutexUnderTest, 1000));
+    CHECK_EQUAL(0, uxSemaphoreGetCount(mMutexUnderTest));
+
+    //now confirm give recursive while here
+    CHECK_EQUAL(pdTRUE, xSemaphoreGiveRecursive(mMutexUnderTest));
+    CHECK_EQUAL(0, uxSemaphoreGetCount(mMutexUnderTest));
+    CHECK_EQUAL(pdTRUE, xSemaphoreGiveRecursive(mMutexUnderTest));
+    CHECK_EQUAL(0, uxSemaphoreGetCount(mMutexUnderTest));
+    CHECK_EQUAL(pdTRUE, xSemaphoreGiveRecursive(mMutexUnderTest));
+    CHECK_EQUAL(1, uxSemaphoreGetCount(mMutexUnderTest));
+}
+
+TEST(MutexTests, library_can_detect_locked_recursive_mutexes)
+{
+    CHECK_FALSE(cms::test::IsAnyMutexLocked());
+    cms::test::MutexTrackingInit();
+    CreateRecursiveMutex();
+    CHECK_FALSE(cms::test::IsAnyMutexLocked());
+
+    xSemaphoreTakeRecursive(mMutexUnderTest, 1000);
+    CHECK_TRUE(cms::test::IsAnyMutexLocked());
+
+    xSemaphoreGiveRecursive(mMutexUnderTest);
+    CHECK_FALSE(cms::test::IsAnyMutexLocked());
+    cms::test::MutexTrackingTeardown();
+}
+
+TEST(MutexTests, library_does_not_detect_locked_recursive_mutex_that_was_deleted)
+{
+    cms::test::MutexTrackingInit();
+    CreateRecursiveMutex();
+    xSemaphoreTakeRecursive(mMutexUnderTest, 1000);
+    CHECK_TRUE(cms::test::IsAnyMutexLocked());
+
     vSemaphoreDelete(mMutexUnderTest);
     mMutexUnderTest = nullptr;
     CHECK_FALSE(cms::test::IsAnyMutexLocked());

Original file line number	Diff line number	Diff line change
`@@ -33,15 +33,19 @@`
`33`	`33`	`extern "C" BaseType_t xQueueSemaphoreTake(QueueHandle_t queue, TickType_t ticks)`
`34`	`34`	`{`
`35`	`35`	`(void) ticks; //ignore in cpputest fake semaphore`
	`36`	`+ configASSERT(queue != nullptr);`
	`37`	`+ configASSERT(queue->queueType != queueQUEUE_TYPE_RECURSIVE_MUTEX);`
`36`	`38`
`37`		`- uint64_t dummy;`
`38`		`- return xQueueReceive(queue, &dummy, ticks);`
	`39`	`+ return cms::InternalQueueReceive(queue);`
`39`	`40`	`}`
`40`	`41`
`41`	`42`
`42`	`43`	`extern "C" BaseType_t xQueueGiveFromISR(QueueHandle_t queue,`
`43`	`44`	`BaseType_t * const pxHigherPriorityTaskWoken)`
`44`	`45`	`{`
	`46`	`+ configASSERT(queue != nullptr);`
	`47`	`+ configASSERT(queue->queueType != queueQUEUE_TYPE_RECURSIVE_MUTEX);`
	`48`	`+`
`45`	`49`	`(void)pxHigherPriorityTaskWoken;`
`46`	`50`	`return xSemaphoreGive(queue);`
`47`	`51`	`}`