From 8ee6e7a8f40f9cd6791d5d3f6239bb13139843cf Mon Sep 17 00:00:00 2001 From: Way Vadhanasin Date: Thu, 27 Apr 2017 11:31:13 -0700 Subject: [PATCH] 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 Signed-off-by: Dan Mihai Reviewed-on: https://gerrit.iotivity.org/gerrit/19605 Tested-by: jenkins-iotivity Reviewed-by: Kevin Kane Reviewed-by: Dmitriy Zhuravlev Reviewed-by: Oleksii Beketov Reviewed-by: Randeep Singh --- resource/c_common/octhread/include/octhread.h | 10 ++ resource/c_common/octhread/src/noop/octhread.c | 5 + resource/c_common/octhread/src/posix/octhread.c | 156 +++++++++++++++++----- resource/c_common/octhread/src/windows/octhread.c | 77 +++++++---- 4 files changed, 195 insertions(+), 53 deletions(-) diff --git a/resource/c_common/octhread/include/octhread.h b/resource/c_common/octhread/include/octhread.h index 713b975..142371b 100644 --- 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 --- 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 --- 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 @@ -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 + /** + * Updating the recursionCount and owner fields must be performed + * while owning the lock, to solve race conditions with other + * threads using the same lock. + */ + if (mutexInfo->recursionCount != 0) + { + oc_mutex_assert_owner(mutex, true); + } + else + { + mutexInfo->owner = oc_get_current_thread_id(); + } + + mutexInfo->recursionCount++; +#endif } 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 + oc_mutex_assert_owner(mutex, true); + + /** + * Updating the recursionCount and owner fields must be performed + * while owning the lock, to solve race conditions with other + * threads using the same lock. + */ + mutexInfo->recursionCount--; + + if (mutexInfo->recursionCount == 0) + { + mutexInfo->owner = OC_INVALID_THREAD_ID; + } +#endif 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 + // Recursively-acquired locks are not supported for use with condition variables. + oc_mutex_assert_owner(mutex, true); + assert(mutexInfo->recursionCount == 1); + mutexInfo->recursionCount = 0; + mutexInfo->owner = OC_INVALID_THREAD_ID; +#endif + // 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 + oc_mutex_assert_owner(mutex, false); + assert(mutexInfo->recursionCount == 0); + mutexInfo->recursionCount = 1; + mutexInfo->owner = oc_get_current_thread_id(); +#endif } switch (ret) @@ -524,17 +613,24 @@ OCWaitResult_t oc_cond_wait_for(oc_cond cond, oc_mutex mutex, uint64_t microseco } else { +#ifndef NDEBUG + // Recursively-acquired locks are not supported for use with condition variables. + oc_mutex_assert_owner(mutex, true); + assert(mutexInfo->recursionCount == 1); + mutexInfo->recursionCount = 0; + mutexInfo->owner = OC_INVALID_THREAD_ID; +#endif + // 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 + oc_mutex_assert_owner(mutex, false); + assert(mutexInfo->recursionCount == 0); + mutexInfo->recursionCount = 1; + mutexInfo->owner = oc_get_current_thread_id(); +#endif } 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 --- 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 + /** + * Updating the recursionCount and owner fields must be performed + * while owning the lock, to solve race conditions with other + * threads using the same lock. + */ + if (mutexInfo->recursionCount != 0) + { + oc_mutex_assert_owner(mutex, true); + } + else + { + mutexInfo->owner = oc_get_current_thread_id(); + } + + mutexInfo->recursionCount++; +#endif } 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 + oc_mutex_assert_owner(mutex, true); + + /** + * Updating the recursionCount and owner fields must be performed + * while owning the lock, to solve race conditions with other + * threads using the same lock. + */ + mutexInfo->recursionCount--; + + if (mutexInfo->recursionCount == 0) + { + mutexInfo->owner = OC_INVALID_THREAD_ID; + } +#endif 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 + // Recursively-acquired locks are not supported for use with condition variables. + oc_mutex_assert_owner(mutex, true); + assert(mutexInfo->recursionCount == 1); + mutexInfo->recursionCount = 0; + mutexInfo->owner = OC_INVALID_THREAD_ID; +#endif // 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 + oc_mutex_assert_owner(mutex, false); + assert(mutexInfo->recursionCount == 0); + mutexInfo->recursionCount = 1; + mutexInfo->owner = oc_get_current_thread_id(); +#endif return retVal; } - -- 2.7.4