IOT-1828 Add oc_mutex_new_recursive

This change adds ability to create oc_mutex with recursive support.

The function oc_mutex_new_recursive() will be used by a later change
in OCStack to remove the responsibility of calling OCProcess from
applications so that they don't have to call it in an endless loop.

Change-Id: I0d60188af9bce866f5ba18800292204c78daba11
Signed-off-by: Way Vadhanasin <wayvad@microsoft.com>
Signed-off-by: Dan Mihai <Daniel.Mihai@microsoft.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/19605
Tested-by: jenkins-iotivity <jenkins@iotivity.org>
Reviewed-by: Kevin Kane <kkane@microsoft.com>
Reviewed-by: Dmitriy Zhuravlev <d.zhuravlev@samsung.com>
Reviewed-by: Oleksii Beketov <ol.beketov@samsung.com>
Reviewed-by: Randeep Singh <randeep.s@samsung.com>

diff --git a/resource/c_common/octhread/include/octhread.h b/resource/c_common/octhread/include/octhread.h
index 713b975..142371b 100644 (file)
--- a/resource/c_common/octhread/include/octhread.h
+++ b/resource/c_common/octhread/include/octhread.h
@@ -111,6 +111,16 @@ OCThreadResult_t oc_thread_wait(oc_thread t);
 oc_mutex oc_mutex_new(void);
 
 /**
+ * Creates new mutex that supports recursion. Use oc_mutex_free to free the created mutex.
+ *
+ * @note The use of recursive mutex in IoTivity is discouraged. Please use it sporadically.
+ *
+ * @return  Reference to newly created mutex, otherwise NULL.
+ *
+ */
+oc_mutex oc_mutex_new_recursive(void);
+
+/**
  * Lock the mutex.
  *
  * @param  mutex  The mutex to be locked.

diff --git a/resource/c_common/octhread/src/noop/octhread.c b/resource/c_common/octhread/src/noop/octhread.c
index 08a565a..b2992b1 100644 (file)
--- a/resource/c_common/octhread/src/noop/octhread.c
+++ b/resource/c_common/octhread/src/noop/octhread.c
@@ -79,6 +79,11 @@ oc_mutex oc_mutex_new(void)
     return (oc_mutex)&g_mutexInfo;
 }
 
+oc_mutex oc_mutex_new_recursive(void)
+{
+    return oc_mutex_new();
+}
+
 bool oc_mutex_free(oc_mutex mutex)
 {
     return true;

diff --git a/resource/c_common/octhread/src/posix/octhread.c b/resource/c_common/octhread/src/posix/octhread.c
index f69a0d5..c87ee80 100644 (file)
--- a/resource/c_common/octhread/src/posix/octhread.c
+++ b/resource/c_common/octhread/src/posix/octhread.c
@@ -37,6 +37,7 @@
 #endif
 
 #include "iotivity_config.h"
+#include "iotivity_debug.h"
 #include "octhread.h"
 #ifdef HAVE_STRING_H
 #include <string.h>
@@ -95,6 +96,7 @@ typedef struct _tagMutexInfo_t
      */
 #ifndef NDEBUG
     pthread_t owner;
+    uint32_t recursionCount;
 #endif
 } oc_mutex_internal;
 
@@ -189,6 +191,7 @@ oc_mutex oc_mutex_new(void)
         {
 #ifndef NDEBUG
             mutexInfo->owner = OC_INVALID_THREAD_ID;
+            mutexInfo->recursionCount = 0;
 #endif
             retVal = (oc_mutex) mutexInfo;
         }
@@ -206,6 +209,66 @@ oc_mutex oc_mutex_new(void)
     return retVal;
 }
 
+oc_mutex oc_mutex_new_recursive(void)
+{
+    oc_mutex retVal = NULL;
+    int ret = -1;
+
+    // Allocate new mutex.
+    oc_mutex_internal *mutexInfo = (oc_mutex_internal*) OICMalloc(sizeof(oc_mutex_internal));
+    if (NULL == mutexInfo)
+    {
+        OIC_LOG_V(ERROR, TAG, "%s Failed to allocate mutex!", __func__);
+        goto exit;
+    }
+
+    // Set up the mutex attributes.
+    pthread_mutexattr_t ma;
+    ret = pthread_mutexattr_init(&ma);
+    if (0 != ret)
+    {
+        OIC_LOG_V(ERROR, TAG, "%s Failed in pthread_mutexattr_init - error %d!",
+            __func__, ret);
+        goto exit;
+    }
+
+    ret = pthread_mutexattr_settype(&ma, PTHREAD_MUTEX_RECURSIVE);
+    if (0 != ret)
+    {
+        OIC_LOG_V(ERROR, TAG, "%s Failed in pthread_mutexattr_settype - error %d!",
+            __func__, ret);
+        pthread_mutexattr_destroy(&ma);
+        goto exit;
+    }
+
+    // Initialize the mutex and destroy the attributes.
+    ret = pthread_mutex_init(&(mutexInfo->mutex), &ma);
+    OC_VERIFY(0 == pthread_mutexattr_destroy(&ma));
+    if (0 != ret)
+    {
+        OIC_LOG_V(ERROR, TAG, "%s Failed in pthread_mutex_init - error %d!",
+            __func__, ret);
+        goto exit;
+    }
+
+#ifndef NDEBUG
+    mutexInfo->owner = OC_INVALID_THREAD_ID;
+    mutexInfo->recursionCount = 0;
+#endif
+
+exit:
+    if (0 == ret)
+    {
+        retVal = (oc_mutex) mutexInfo;
+    }
+    else
+    {
+        OICFree(mutexInfo);
+    }
+
+    return retVal;
+}
+
 bool oc_mutex_free(oc_mutex mutex)
 {
     bool bRet=false;
@@ -243,13 +306,23 @@ void oc_mutex_lock(oc_mutex mutex)
             exit(ret);
         }
 
-#ifndef NDEBUG
-        /**
-         * Updating the owner field must be performed while owning the lock,
-         * to solve race conditions with other threads using the same lock.
-         */
-        mutexInfo->owner = oc_get_current_thread_id();
-#endif
+#ifndef NDEBUG\r
+        /**\r
+         * Updating the recursionCount and owner fields must be performed\r
+         * while owning the lock, to solve race conditions with other\r
+         * threads using the same lock.\r
+         */\r
+        if (mutexInfo->recursionCount != 0)\r
+        {\r
+            oc_mutex_assert_owner(mutex, true);\r
+        }\r
+        else\r
+        {\r
+            mutexInfo->owner = oc_get_current_thread_id();\r
+        }\r
+\r
+        mutexInfo->recursionCount++;\r
+#endif\r
     }
     else
     {
@@ -262,13 +335,21 @@ void oc_mutex_unlock(oc_mutex mutex)
     oc_mutex_internal *mutexInfo = (oc_mutex_internal*) mutex;
     if (mutexInfo)
     {
-#ifndef NDEBUG
-        /**
-         * Updating the owner field must be performed while owning the lock,
-         * to solve race conditions with other threads using the same lock.
-         */
-        mutexInfo->owner = OC_INVALID_THREAD_ID;
-#endif
+#ifndef NDEBUG\r
+        oc_mutex_assert_owner(mutex, true);\r
+\r
+        /**\r
+         * Updating the recursionCount and owner fields must be performed\r
+         * while owning the lock, to solve race conditions with other\r
+         * threads using the same lock.\r
+         */\r
+        mutexInfo->recursionCount--;\r
+\r
+        if (mutexInfo->recursionCount == 0)\r
+        {\r
+            mutexInfo->owner = OC_INVALID_THREAD_ID;\r
+        }\r
+#endif\r
 
         int ret = pthread_mutex_unlock(&mutexInfo->mutex);
         if(ret != 0)
@@ -297,6 +378,7 @@ void oc_mutex_assert_owner(const oc_mutex mutex, bool currentThreadIsOwner)
     if (currentThreadIsOwner)
     {
         assert(pthread_equal(mutexInfo->owner, currentThreadID));
+        assert(mutexInfo->recursionCount != 0);
     }
     else
     {
@@ -491,16 +573,23 @@ OCWaitResult_t oc_cond_wait_for(oc_cond cond, oc_mutex mutex, uint64_t microseco
              abstime = oc_get_current_time();
             oc_add_microseconds_to_timespec(&abstime, microseconds);
 
+#ifndef NDEBUG\r
+            // Recursively-acquired locks are not supported for use with condition variables.\r
+            oc_mutex_assert_owner(mutex, true);\r
+            assert(mutexInfo->recursionCount == 1);\r
+            mutexInfo->recursionCount = 0;\r
+            mutexInfo->owner = OC_INVALID_THREAD_ID;\r
+#endif\r
+
             // Wait for the given time
-#ifndef NDEBUG
-            // The conditional variable wait API used will atomically release the mutex, but the
-            // best we can do here is to just clear the owner info before the API is called.
-            mutexInfo->owner = OC_INVALID_THREAD_ID;
-#endif
             ret = pthread_cond_timedwait(&(eventInfo->cond), &(mutexInfo->mutex), &abstime);
-#ifndef NDEBUG
-            mutexInfo->owner = oc_get_current_thread_id();
-#endif
+
+#ifndef NDEBUG\r
+            oc_mutex_assert_owner(mutex, false);\r
+            assert(mutexInfo->recursionCount == 0);\r
+            mutexInfo->recursionCount = 1;\r
+            mutexInfo->owner = oc_get_current_thread_id();\r
+#endif\r
         }
 
         switch (ret)
@@ -524,17 +613,24 @@ OCWaitResult_t oc_cond_wait_for(oc_cond cond, oc_mutex mutex, uint64_t microseco
     }
     else
     {
+#ifndef NDEBUG\r
+        // Recursively-acquired locks are not supported for use with condition variables.\r
+        oc_mutex_assert_owner(mutex, true);\r
+        assert(mutexInfo->recursionCount == 1);\r
+        mutexInfo->recursionCount = 0;\r
+        mutexInfo->owner = OC_INVALID_THREAD_ID;\r
+#endif\r
+
         // Wait forever
-#ifndef NDEBUG
-        // The conditional variable wait API used will atomically release the mutex, but the
-        // best we can do here is to just clear the owner info before the API is called.
-        mutexInfo->owner = OC_INVALID_THREAD_ID;
-#endif
         int ret = pthread_cond_wait(&eventInfo->cond, &mutexInfo->mutex);
         retVal = (ret == 0) ? OC_WAIT_SUCCESS : OC_WAIT_INVAL;
-#ifndef NDEBUG
-        mutexInfo->owner = oc_get_current_thread_id();
-#endif
+
+#ifndef NDEBUG\r
+        oc_mutex_assert_owner(mutex, false);\r
+        assert(mutexInfo->recursionCount == 0);\r
+        mutexInfo->recursionCount = 1;\r
+        mutexInfo->owner = oc_get_current_thread_id();\r
+#endif\r
     }
     return retVal;
 }

diff --git a/resource/c_common/octhread/src/windows/octhread.c b/resource/c_common/octhread/src/windows/octhread.c
index 656ae40..cef1b06 100644 (file)
--- a/resource/c_common/octhread/src/windows/octhread.c
+++ b/resource/c_common/octhread/src/windows/octhread.c
@@ -48,6 +48,7 @@ typedef struct _tagMutexInfo_t
      */
 #ifndef NDEBUG
     DWORD owner;
+    uint32_t recursionCount;
 #endif
 } oc_mutex_internal;
 
@@ -138,6 +139,7 @@ oc_mutex oc_mutex_new(void)
     {
 #ifndef NDEBUG
         mutexInfo->owner = OC_INVALID_THREAD_ID;
+        mutexInfo->recursionCount = 0;
 #endif
         InitializeCriticalSection(&mutexInfo->mutex);
         retVal = (oc_mutex)mutexInfo;
@@ -150,6 +152,11 @@ oc_mutex oc_mutex_new(void)
     return retVal;
 }
 
+oc_mutex oc_mutex_new_recursive(void)
+{
+    return oc_mutex_new();
+}
+
 bool oc_mutex_free(oc_mutex mutex)
 {
     bool bRet = false;
@@ -175,13 +182,23 @@ void oc_mutex_lock(oc_mutex mutex)
     {
         EnterCriticalSection(&mutexInfo->mutex);
 
-#ifndef NDEBUG
-        /**
-         * Updating the owner field must be performed while owning the lock,
-         * to solve race conditions with other threads using the same lock.
-         */
-        mutexInfo->owner = oc_get_current_thread_id();
-#endif
+#ifndef NDEBUG\r
+        /**\r
+         * Updating the recursionCount and owner fields must be performed\r
+         * while owning the lock, to solve race conditions with other\r
+         * threads using the same lock.\r
+         */\r
+        if (mutexInfo->recursionCount != 0)\r
+        {\r
+            oc_mutex_assert_owner(mutex, true);\r
+        }\r
+        else\r
+        {\r
+            mutexInfo->owner = oc_get_current_thread_id();\r
+        }\r
+\r
+        mutexInfo->recursionCount++;\r
+#endif\r
     }
     else
     {
@@ -192,15 +209,24 @@ void oc_mutex_lock(oc_mutex mutex)
 void oc_mutex_unlock(oc_mutex mutex)
 {
     oc_mutex_internal *mutexInfo = (oc_mutex_internal*) mutex;
+
     if (mutexInfo)
     {
-#ifndef NDEBUG
-        /**
-         * Updating the owner field must be performed while owning the lock,
-         * to solve race conditions with other threads using the same lock.
-         */
-        mutexInfo->owner = OC_INVALID_THREAD_ID;
-#endif
+#ifndef NDEBUG\r
+        oc_mutex_assert_owner(mutex, true);\r
+\r
+        /**\r
+         * Updating the recursionCount and owner fields must be performed\r
+         * while owning the lock, to solve race conditions with other\r
+         * threads using the same lock.\r
+         */\r
+        mutexInfo->recursionCount--;\r
+\r
+        if (mutexInfo->recursionCount == 0)\r
+        {\r
+            mutexInfo->owner = OC_INVALID_THREAD_ID;\r
+        }\r
+#endif\r
 
         LeaveCriticalSection(&mutexInfo->mutex);
     }
@@ -223,6 +249,7 @@ void oc_mutex_assert_owner(const oc_mutex mutex, bool currentThreadIsOwner)
     if (currentThreadIsOwner)
     {
         assert(mutexInfo->owner == currentThreadID);
+        assert(mutexInfo->recursionCount != 0);
     }
     else
     {
@@ -321,11 +348,13 @@ OCWaitResult_t oc_cond_wait_for(oc_cond cond, oc_mutex mutex, uint64_t microseco
         milli = INFINITE;
     }
 
-#ifndef NDEBUG
-    // The conditional variable wait API used will atomically release the mutex, but the
-    // best we can do here is to just clear the owner info before the API is called.
-    mutexInfo->owner = OC_INVALID_THREAD_ID;
-#endif
+#ifndef NDEBUG\r
+    // Recursively-acquired locks are not supported for use with condition variables.\r
+    oc_mutex_assert_owner(mutex, true);\r
+    assert(mutexInfo->recursionCount == 1);\r
+    mutexInfo->recursionCount = 0;\r
+    mutexInfo->owner = OC_INVALID_THREAD_ID;\r
+#endif\r
 
     // Wait for the given time        
     if (!SleepConditionVariableCS(&eventInfo->cond, &mutexInfo->mutex, milli))
@@ -345,10 +374,12 @@ OCWaitResult_t oc_cond_wait_for(oc_cond cond, oc_mutex mutex, uint64_t microseco
         retVal = OC_WAIT_SUCCESS;
     }
 
-#ifndef NDEBUG
-    mutexInfo->owner = oc_get_current_thread_id();
-#endif
+#ifndef NDEBUG\r
+    oc_mutex_assert_owner(mutex, false);\r
+    assert(mutexInfo->recursionCount == 0);\r
+    mutexInfo->recursionCount = 1;\r
+    mutexInfo->owner = oc_get_current_thread_id();\r
+#endif\r
 
     return retVal;
 }
-