Skip to content

Bind condvars to external mutex for POSIX waits #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
francdoc wants to merge 1 commit into from
Closed

Bind condvars to external mutex for POSIX waits #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/os/portable/os-impl-no-condvar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 0 additions & 1 deletion src/os/posix/inc/os-impl-condvar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
/* CondVares */
typedef struct
{
pthread_mutex_t mut;
pthread_cond_t cv;
} OS_impl_condvar_internal_record_t;

Expand Down
126 changes: 61 additions & 65 deletions src/os/posix/src/os-impl-condvar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
***************************************************************************************/
Expand All @@ -67,40 +59,23 @@
*-----------------------------------------------------------------*/
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;
}

/*----------------------------------------------------------------
Expand All @@ -111,26 +86,18 @@
*-----------------------------------------------------------------*/
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;
}

/*----------------------------------------------------------------
Expand All @@ -142,11 +109,20 @@
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)
Comment on lines +117 to +118

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
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;
Expand All @@ -164,11 +140,20 @@
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)
Comment on lines +148 to +149

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
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;
Expand Down Expand Up @@ -230,20 +215,22 @@
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)
Comment on lines +224 to +225

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
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;
Expand All @@ -262,16 +249,25 @@
{
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)
Comment on lines +258 to +259

Check warning

Code scanning / CodeQL

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
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)
{
Expand Down
37 changes: 28 additions & 9 deletions src/tests/condvar-test/condvar-test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

/*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand All @@ -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),
Expand All @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions src/unit-test-coverage/portable/src/coveragetest-no-condvar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 9 additions & 5 deletions src/unit-test-coverage/shared/src/coveragetest-condvar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/unit-test-coverage/ut-stubs/src/os-shared-condvar-impl-stubs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading
Loading