diff --git a/src/os/portable/os-impl-no-condvar.c b/src/os/portable/os-impl-no-condvar.c index c0070737c..bf26c8bcd 100644 --- a/src/os/portable/os-impl-no-condvar.c +++ b/src/os/portable/os-impl-no-condvar.c @@ -33,6 +33,9 @@ int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, osal_id_t mutex_id, uint32 options) { + (void)token; + (void)mutex_id; + (void)options; return OS_ERR_NOT_IMPLEMENTED; } diff --git a/src/os/posix/inc/os-impl-condvar.h b/src/os/posix/inc/os-impl-condvar.h index 85382b558..3f0f1e9cf 100644 --- a/src/os/posix/inc/os-impl-condvar.h +++ b/src/os/posix/inc/os-impl-condvar.h @@ -32,7 +32,6 @@ /* CondVares */ typedef struct { - pthread_mutex_t mut; pthread_cond_t cv; } OS_impl_condvar_internal_record_t; diff --git a/src/os/posix/src/os-impl-condvar.c b/src/os/posix/src/os-impl-condvar.c index f5e7bb2d8..f7c670354 100644 --- a/src/os/posix/src/os-impl-condvar.c +++ b/src/os/posix/src/os-impl-condvar.c @@ -31,19 +31,11 @@ #include "os-shared-condvar.h" #include "os-shared-idmap.h" #include "os-impl-condvar.h" +#include "os-impl-mutex.h" /* Tables where the OS object information is stored */ OS_impl_condvar_internal_record_t OS_impl_condvar_table[OS_MAX_CONDVARS]; -/*--------------------------------------------------------------------------------------- - * Helper function for releasing the mutex in case the thread - * executing pthread_cond_wait() is canceled. - ----------------------------------------------------------------------------------------*/ -static void OS_Posix_CondVarReleaseMutex(void *mut) -{ - pthread_mutex_unlock(mut); -} - /**************************************************************************************** CONDVAR API ***************************************************************************************/ @@ -67,40 +59,23 @@ int32 OS_Posix_CondVarAPI_Impl_Init(void) *-----------------------------------------------------------------*/ int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, osal_id_t mutex_id, uint32 options) { - int32 final_status; int status; OS_impl_condvar_internal_record_t *impl; - final_status = OS_SUCCESS; - impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + (void)mutex_id; + (void)options; + + impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); - /* - ** create the underlying mutex - */ - status = pthread_mutex_init(&impl->mut, NULL); + status = pthread_cond_init(&impl->cv, NULL); if (status != 0) { - OS_DEBUG("Error: CondVar mutex could not be created. ID = %lu: %s\n", + OS_DEBUG("Error: CondVar could not be created. ID = %lu: %s\n", OS_ObjectIdToInteger(OS_ObjectIdFromToken(token)), strerror(status)); - final_status = OS_ERROR; - } - else - { - /* - ** create the condvar - */ - status = pthread_cond_init(&impl->cv, NULL); - if (status != 0) - { - pthread_mutex_destroy(&impl->mut); - - OS_DEBUG("Error: CondVar could not be created. ID = %lu: %s\n", - OS_ObjectIdToInteger(OS_ObjectIdFromToken(token)), strerror(status)); - final_status = OS_ERROR; - } + return OS_ERROR; } - return final_status; + return OS_SUCCESS; } /*---------------------------------------------------------------- @@ -111,26 +86,18 @@ int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, osal_id_t mutex_id, *-----------------------------------------------------------------*/ int32 OS_CondVarDelete_Impl(const OS_object_token_t *token) { - int32 final_status; int status; OS_impl_condvar_internal_record_t *impl; - final_status = OS_SUCCESS; - impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); status = pthread_cond_destroy(&impl->cv); if (status != 0) { - final_status = OS_ERROR; - } - - status = pthread_mutex_destroy(&impl->mut); - if (status != 0) - { - final_status = OS_ERROR; + return OS_ERROR; } - return final_status; + return OS_SUCCESS; } /*---------------------------------------------------------------- @@ -142,11 +109,20 @@ int32 OS_CondVarDelete_Impl(const OS_object_token_t *token) int32 OS_CondVarUnlock_Impl(const OS_object_token_t *token) { int status; - OS_impl_condvar_internal_record_t *impl; + OS_condvar_internal_record_t * condvar; + OS_impl_mutex_internal_record_t * mutex_impl; + OS_object_token_t mutex_token; - impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + condvar = OS_OBJECT_TABLE_GET(OS_condvar_table, *token); + if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_MUTEX, condvar->bound_mutex, &mutex_token) != + OS_SUCCESS) + { + return OS_ERR_INVALID_ID; + } + + mutex_impl = OS_OBJECT_TABLE_GET(OS_impl_mutex_table, mutex_token); - status = pthread_mutex_unlock(&impl->mut); + status = pthread_mutex_unlock(&mutex_impl->id); if (status != 0) { return OS_ERROR; @@ -164,11 +140,20 @@ int32 OS_CondVarUnlock_Impl(const OS_object_token_t *token) int32 OS_CondVarLock_Impl(const OS_object_token_t *token) { int status; - OS_impl_condvar_internal_record_t *impl; + OS_condvar_internal_record_t * condvar; + OS_impl_mutex_internal_record_t * mutex_impl; + OS_object_token_t mutex_token; - impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + condvar = OS_OBJECT_TABLE_GET(OS_condvar_table, *token); + if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_MUTEX, condvar->bound_mutex, &mutex_token) != + OS_SUCCESS) + { + return OS_ERR_INVALID_ID; + } - status = pthread_mutex_lock(&impl->mut); + mutex_impl = OS_OBJECT_TABLE_GET(OS_impl_mutex_table, mutex_token); + + status = pthread_mutex_lock(&mutex_impl->id); if (status != 0) { return OS_ERROR; @@ -230,20 +215,22 @@ int32 OS_CondVarBroadcast_Impl(const OS_object_token_t *token) int32 OS_CondVarWait_Impl(const OS_object_token_t *token) { int status; + OS_condvar_internal_record_t * condvar; OS_impl_condvar_internal_record_t *impl; + OS_impl_mutex_internal_record_t * mutex_impl; + OS_object_token_t mutex_token; - impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + condvar = OS_OBJECT_TABLE_GET(OS_condvar_table, *token); + if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_MUTEX, condvar->bound_mutex, &mutex_token) != + OS_SUCCESS) + { + return OS_ERR_INVALID_ID; + } - /* - * note that because pthread_cond_wait is a cancellation point, this needs to - * employ the same protection that is in the binsem module. In the event that - * the thread is canceled inside pthread_cond_wait, the mutex will be re-acquired - * before the cancellation occurs, leaving the mutex in a locked state. - */ - pthread_cleanup_push(OS_Posix_CondVarReleaseMutex, &impl->mut); - status = pthread_cond_wait(&impl->cv, &impl->mut); - pthread_cleanup_pop(false); + impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + mutex_impl = OS_OBJECT_TABLE_GET(OS_impl_mutex_table, mutex_token); + status = pthread_cond_wait(&impl->cv, &mutex_impl->id); if (status != 0) { return OS_ERROR; @@ -262,16 +249,25 @@ int32 OS_CondVarTimedWait_Impl(const OS_object_token_t *token, const OS_time_t * { struct timespec limit; int status; + OS_condvar_internal_record_t * condvar; OS_impl_condvar_internal_record_t *impl; + OS_impl_mutex_internal_record_t * mutex_impl; + OS_object_token_t mutex_token; - impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + condvar = OS_OBJECT_TABLE_GET(OS_condvar_table, *token); + if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_MUTEX, condvar->bound_mutex, &mutex_token) != + OS_SUCCESS) + { + return OS_ERR_INVALID_ID; + } + + impl = OS_OBJECT_TABLE_GET(OS_impl_condvar_table, *token); + mutex_impl = OS_OBJECT_TABLE_GET(OS_impl_mutex_table, mutex_token); limit.tv_sec = OS_TimeGetTotalSeconds(*abs_wakeup_time); limit.tv_nsec = OS_TimeGetNanosecondsPart(*abs_wakeup_time); - pthread_cleanup_push(OS_Posix_CondVarReleaseMutex, &impl->mut); - status = pthread_cond_timedwait(&impl->cv, &impl->mut, &limit); - pthread_cleanup_pop(false); + status = pthread_cond_timedwait(&impl->cv, &mutex_impl->id, &limit); if (status == ETIMEDOUT) { diff --git a/src/tests/condvar-test/condvar-test.c b/src/tests/condvar-test/condvar-test.c index c5a583498..d8fb73882 100644 --- a/src/tests/condvar-test/condvar-test.c +++ b/src/tests/condvar-test/condvar-test.c @@ -54,6 +54,7 @@ condvar_task_stack_t task_stacks[NUM_TASKS]; condvar_task_state_t task_states[NUM_TASKS]; osal_id_t condvar_id; +osal_id_t condvar_mutex_id; uint32 curr_condition; uint32 total_work; @@ -120,9 +121,10 @@ void CondVarTest_Setup(void) total_work = 0; /* - ** Create the condvar + ** Create the mutex and condvar */ - UtAssert_INT32_EQ(OS_CondVarCreate(&condvar_id, "CondVar", 0), OS_SUCCESS); + UtAssert_INT32_EQ(OS_MutSemCreate(&condvar_mutex_id, "CondVarMtx", 0), OS_SUCCESS); + UtAssert_INT32_EQ(OS_CondVarCreate(&condvar_id, "CondVar", condvar_mutex_id, 0), OS_SUCCESS); UtPrintf("CondVar create Id=%ld", OS_ObjectIdToInteger(condvar_id)); /* @@ -231,6 +233,7 @@ void CondVarTest_Teardown(void) UtAssert_INT32_EQ(OS_CondVarUnlock(condvar_id), OS_SUCCESS); UtAssert_INT32_EQ(OS_CondVarDelete(condvar_id), OS_SUCCESS); + UtAssert_INT32_EQ(OS_MutSemDelete(condvar_mutex_id), OS_SUCCESS); } OS_time_t testtm_ref1; @@ -279,7 +282,8 @@ void CondVarTimedWait_Setup(void) { total_work = 0; - UtAssert_INT32_EQ(OS_CondVarCreate(&condvar_id, "CondVar", 0), OS_SUCCESS); + UtAssert_INT32_EQ(OS_MutSemCreate(&condvar_mutex_id, "CondVarMtx", 0), OS_SUCCESS); + UtAssert_INT32_EQ(OS_CondVarCreate(&condvar_id, "CondVar", condvar_mutex_id, 0), OS_SUCCESS); UtAssert_INT32_EQ(OS_TaskCreate(&task_states[0].task_id, "timedwait", condvar_timedtask_entry, OSAL_STACKPTR_C(&task_stacks[0]), sizeof(task_stacks[0]), OSAL_PRIORITY_C(10), 0), @@ -326,6 +330,7 @@ void CondVarTimedWait_Teardown(void) UtAssert_INT32_EQ(OS_CondVarUnlock(condvar_id), OS_SUCCESS); UtAssert_INT32_EQ(OS_CondVarDelete(condvar_id), OS_SUCCESS); + UtAssert_INT32_EQ(OS_MutSemDelete(condvar_mutex_id), OS_SUCCESS); } void CondVarTest_Ops(void) @@ -334,20 +339,23 @@ void CondVarTest_Ops(void) char cv_name[OS_MAX_API_NAME]; osal_id_t cv_id[OS_MAX_CONDVARS]; osal_id_t cv_extra; + osal_id_t test_mutex; OS_condvar_prop_t cv_prop; - UtAssert_INT32_EQ(OS_CondVarCreate(NULL, "cvex", 0), OS_INVALID_POINTER); - UtAssert_INT32_EQ(OS_CondVarCreate(&cv_extra, NULL, 0), OS_INVALID_POINTER); + UtAssert_INT32_EQ(OS_MutSemCreate(&test_mutex, "cvMut", 0), OS_SUCCESS); + + UtAssert_INT32_EQ(OS_CondVarCreate(NULL, "cvex", test_mutex, 0), OS_INVALID_POINTER); + UtAssert_INT32_EQ(OS_CondVarCreate(&cv_extra, NULL, test_mutex, 0), OS_INVALID_POINTER); for (i = 0; i < OS_MAX_CONDVARS; ++i) { snprintf(cv_name, sizeof(cv_name), "cv%03u", (unsigned int)i); - UtAssert_INT32_EQ(OS_CondVarCreate(&cv_id[i], cv_name, 0), OS_SUCCESS); + UtAssert_INT32_EQ(OS_CondVarCreate(&cv_id[i], cv_name, test_mutex, 0), OS_SUCCESS); } - UtAssert_INT32_EQ(OS_CondVarCreate(&cv_extra, "cvex", 0), OS_ERR_NO_FREE_IDS); + UtAssert_INT32_EQ(OS_CondVarCreate(&cv_extra, "cvex", test_mutex, 0), OS_ERR_NO_FREE_IDS); UtAssert_INT32_EQ(OS_CondVarDelete(cv_id[OS_MAX_CONDVARS - 1]), OS_SUCCESS); - UtAssert_INT32_EQ(OS_CondVarCreate(&cv_extra, "cv000", 0), OS_ERR_NAME_TAKEN); + UtAssert_INT32_EQ(OS_CondVarCreate(&cv_extra, "cv000", test_mutex, 0), OS_ERR_NAME_TAKEN); UtAssert_INT32_EQ(OS_CondVarGetIdByName(&cv_extra, "cv000"), OS_SUCCESS); UtAssert_True(OS_ObjectIdEqual(cv_extra, cv_id[0]), "objid (%lu) == cv_id[0] (%lu)", OS_ObjectIdToInteger(cv_extra), @@ -364,19 +372,30 @@ void CondVarTest_Ops(void) snprintf(cv_name, sizeof(cv_name), "cv%03u", (unsigned int)i); UtAssert_INT32_EQ(OS_CondVarDelete(cv_id[i]), OS_SUCCESS); } + + UtAssert_INT32_EQ(OS_MutSemDelete(test_mutex), OS_SUCCESS); } bool CondVarTest_CheckImpl(void) { int32_t status; osal_id_t cvid; + osal_id_t test_mutex; + + status = OS_MutSemCreate(&test_mutex, "cvChk", 0); + if (status != OS_SUCCESS) + { + return false; + } - status = OS_CondVarCreate(&cvid, "ut", 0); + status = OS_CondVarCreate(&cvid, "ut", test_mutex, 0); if (status == OS_SUCCESS) { OS_CondVarDelete(cvid); } + OS_MutSemDelete(test_mutex); + return (status != OS_ERR_NOT_IMPLEMENTED); } diff --git a/src/unit-test-coverage/portable/src/coveragetest-no-condvar.c b/src/unit-test-coverage/portable/src/coveragetest-no-condvar.c index 75aecff6c..9a8654d87 100644 --- a/src/unit-test-coverage/portable/src/coveragetest-no-condvar.c +++ b/src/unit-test-coverage/portable/src/coveragetest-no-condvar.c @@ -29,10 +29,10 @@ void Test_OS_CondVarCreate_Impl(void) { /* Test Case For: - * int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, uint32 options) + * int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, osal_id_t mutex_id, uint32 options) */ - OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate_Impl, (UT_INDEX_0, 0), OS_ERR_NOT_IMPLEMENTED); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate_Impl, (UT_INDEX_0, UT_OBJID_1, 0), OS_ERR_NOT_IMPLEMENTED); } void Test_OS_CondVarLock_Impl(void) diff --git a/src/unit-test-coverage/shared/src/coveragetest-condvar.c b/src/unit-test-coverage/shared/src/coveragetest-condvar.c index 326ec9dbf..60c41be9a 100644 --- a/src/unit-test-coverage/shared/src/coveragetest-condvar.c +++ b/src/unit-test-coverage/shared/src/coveragetest-condvar.c @@ -48,17 +48,21 @@ void Test_OS_CondVarCreate(void) */ osal_id_t objid = OS_OBJECT_ID_UNDEFINED; - OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", 0), OS_SUCCESS); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", UT_OBJID_1, 0), OS_SUCCESS); OSAPI_TEST_OBJID(objid, !=, OS_OBJECT_ID_UNDEFINED); - OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(NULL, "UT", 0), OS_INVALID_POINTER); - OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, NULL, 0), OS_INVALID_POINTER); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(NULL, "UT", UT_OBJID_1, 0), OS_INVALID_POINTER); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, NULL, UT_OBJID_1, 0), OS_INVALID_POINTER); + + UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdGetById), OS_ERR_INVALID_ID); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", UT_OBJID_1, 0), OS_ERR_INVALID_ID); + UT_ClearDefaultReturnValue(UT_KEY(OS_ObjectIdGetById)); UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdAllocateNew), OS_ERROR); - OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", 0), OS_ERROR); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", UT_OBJID_1, 0), OS_ERROR); UT_SetDefaultReturnValue(UT_KEY(OCS_memchr), OS_ERROR); - OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", 0), OS_ERR_NAME_TOO_LONG); + OSAPI_TEST_FUNCTION_RC(OS_CondVarCreate(&objid, "UT", UT_OBJID_1, 0), OS_ERR_NAME_TOO_LONG); } void Test_OS_CondVarDelete(void) diff --git a/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-impl-stubs.c b/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-impl-stubs.c index e189ce562..acbe554a2 100644 --- a/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-impl-stubs.c +++ b/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-impl-stubs.c @@ -46,11 +46,12 @@ int32 OS_CondVarBroadcast_Impl(const OS_object_token_t *token) * Generated stub function for OS_CondVarCreate_Impl() * ---------------------------------------------------- */ -int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, uint32 options) +int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, osal_id_t mutex_id, uint32 options) { UT_GenStub_SetupReturnBuffer(OS_CondVarCreate_Impl, int32); UT_GenStub_AddParam(OS_CondVarCreate_Impl, const OS_object_token_t *, token); + UT_GenStub_AddParam(OS_CondVarCreate_Impl, osal_id_t, mutex_id); UT_GenStub_AddParam(OS_CondVarCreate_Impl, uint32, options); UT_GenStub_Execute(OS_CondVarCreate_Impl, Basic, NULL); diff --git a/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-stubs.c b/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-stubs.c index 2827a930e..0f93a4dda 100644 --- a/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-stubs.c +++ b/src/unit-test-coverage/ut-stubs/src/os-shared-condvar-stubs.c @@ -60,11 +60,12 @@ int32 OS_CondVarBroadcast_Impl(const OS_object_token_t *token) * Generated stub function for OS_CondVarCreate_Impl() * ---------------------------------------------------- */ -int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, uint32 options) +int32 OS_CondVarCreate_Impl(const OS_object_token_t *token, osal_id_t mutex_id, uint32 options) { UT_GenStub_SetupReturnBuffer(OS_CondVarCreate_Impl, int32); UT_GenStub_AddParam(OS_CondVarCreate_Impl, const OS_object_token_t *, token); + UT_GenStub_AddParam(OS_CondVarCreate_Impl, osal_id_t, mutex_id); UT_GenStub_AddParam(OS_CondVarCreate_Impl, uint32, options); UT_GenStub_Execute(OS_CondVarCreate_Impl, Basic, NULL); diff --git a/src/ut-stubs/osapi-condvar-stubs.c b/src/ut-stubs/osapi-condvar-stubs.c index 01b4738e4..bb5411111 100644 --- a/src/ut-stubs/osapi-condvar-stubs.c +++ b/src/ut-stubs/osapi-condvar-stubs.c @@ -52,6 +52,7 @@ int32 OS_CondVarCreate(osal_id_t *var_id, const char *var_name, osal_id_t mutex_ UT_GenStub_AddParam(OS_CondVarCreate, osal_id_t *, var_id); UT_GenStub_AddParam(OS_CondVarCreate, const char *, var_name); + UT_GenStub_AddParam(OS_CondVarCreate, osal_id_t, mutex_id); UT_GenStub_AddParam(OS_CondVarCreate, uint32, options); UT_GenStub_Execute(OS_CondVarCreate, Basic, NULL);