Skip to content

Commit

Permalink
Merge pull request #1311 from jphickey/fix-1304-condvar-lock
Browse files Browse the repository at this point in the history 
Fix #1304, locks for condvar objects on rtems/vxworks
  • Loading branch information
@dzbaker
dzbaker committed Oct 17, 2022
2 parents 0bd6c42 + 954f053 commit 7bd5036
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 26 deletions.
32 changes: 19 additions & 13 deletions src/os/posix/src/os-impl-idmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,13 @@ void OS_Lock_Global_Impl(osal_objtype_t idtype)

impl = OS_impl_objtype_lock_table[idtype];

ret = pthread_mutex_lock(&impl->mutex);
if (ret != 0)
if (impl != NULL)
{
OS_DEBUG("pthread_mutex_lock(&impl->mutex): %s", strerror(ret));
ret = pthread_mutex_lock(&impl->mutex);
if (ret != 0)
{
OS_DEBUG("pthread_mutex_lock(&impl->mutex): %s", strerror(ret));
}
}

} /* end OS_Lock_Global_Impl */
Expand All @@ -112,18 +115,21 @@ void OS_Unlock_Global_Impl(osal_objtype_t idtype)

impl = OS_impl_objtype_lock_table[idtype];

/* Notify any waiting threads that the state _may_ have changed */
ret = pthread_cond_broadcast(&impl->cond);
if (ret != 0)
if (impl != NULL)
{
OS_DEBUG("pthread_cond_broadcast(&impl->cond): %s", strerror(ret));
/* unexpected but keep going (not critical) */
}
/* Notify any waiting threads that the state _may_ have changed */
ret = pthread_cond_broadcast(&impl->cond);
if (ret != 0)
{
OS_DEBUG("pthread_cond_broadcast(&impl->cond): %s", strerror(ret));
/* unexpected but keep going (not critical) */
}

ret = pthread_mutex_unlock(&impl->mutex);
if (ret != 0)
{
OS_DEBUG("pthread_mutex_unlock(&impl->mutex): %s", strerror(ret));
ret = pthread_mutex_unlock(&impl->mutex);
if (ret != 0)
{
OS_DEBUG("pthread_mutex_unlock(&impl->mutex): %s", strerror(ret));
}
}

} /* end OS_Unlock_Global_Impl */
Expand Down
20 changes: 14 additions & 6 deletions src/os/rtems/src/os-impl-idmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ static OS_impl_objtype_lock_t OS_timecb_table_lock;
static OS_impl_objtype_lock_t OS_module_table_lock;
static OS_impl_objtype_lock_t OS_filesys_table_lock;
static OS_impl_objtype_lock_t OS_console_lock;
static OS_impl_objtype_lock_t OS_condvar_lock;

OS_impl_objtype_lock_t *const OS_impl_objtype_lock_table[OS_OBJECT_TYPE_USER] = {
[OS_OBJECT_TYPE_UNDEFINED] = NULL,
Expand All @@ -70,6 +71,7 @@ OS_impl_objtype_lock_t *const OS_impl_objtype_lock_table[OS_OBJECT_TYPE_USER] =
[OS_OBJECT_TYPE_OS_MODULE] = &OS_module_table_lock,
[OS_OBJECT_TYPE_OS_FILESYS] = &OS_filesys_table_lock,
[OS_OBJECT_TYPE_OS_CONSOLE] = &OS_console_lock,
[OS_OBJECT_TYPE_OS_CONDVAR] = &OS_condvar_lock,
};

/*----------------------------------------------------------------
Expand All @@ -87,10 +89,13 @@ void OS_Lock_Global_Impl(osal_objtype_t idtype)

impl = OS_impl_objtype_lock_table[idtype];

rtems_sc = rtems_semaphore_obtain(impl->id, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
if (rtems_sc != RTEMS_SUCCESSFUL)
if (impl != NULL)
{
OS_DEBUG("OS_Lock_Global_Impl: rtems_semaphore_obtain failed: %s\n", rtems_status_text(rtems_sc));
rtems_sc = rtems_semaphore_obtain(impl->id, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
if (rtems_sc != RTEMS_SUCCESSFUL)
{
OS_DEBUG("OS_Lock_Global_Impl: rtems_semaphore_obtain failed: %s\n", rtems_status_text(rtems_sc));
}
}

} /* end OS_Lock_Global_Impl */
Expand All @@ -110,10 +115,13 @@ void OS_Unlock_Global_Impl(osal_objtype_t idtype)

impl = OS_impl_objtype_lock_table[idtype];

rtems_sc = rtems_semaphore_release(impl->id);
if (rtems_sc != RTEMS_SUCCESSFUL)
if (impl != NULL)
{
OS_DEBUG("OS_Unlock_Global_Impl: rtems_semaphore_release failed: %s\n", rtems_status_text(rtems_sc));
rtems_sc = rtems_semaphore_release(impl->id);
if (rtems_sc != RTEMS_SUCCESSFUL)
{
OS_DEBUG("OS_Unlock_Global_Impl: rtems_semaphore_release failed: %s\n", rtems_status_text(rtems_sc));
}
}

} /* end OS_Unlock_Global_Impl */
Expand Down
19 changes: 14 additions & 5 deletions src/os/vxworks/src/os-impl-idmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ VX_MUTEX_SEMAPHORE(OS_timecb_table_mut_mem);
VX_MUTEX_SEMAPHORE(OS_module_table_mut_mem);
VX_MUTEX_SEMAPHORE(OS_filesys_table_mut_mem);
VX_MUTEX_SEMAPHORE(OS_console_table_mut_mem);
VX_MUTEX_SEMAPHORE(OS_condvar_table_mut_mem);

static OS_impl_objtype_lock_t OS_task_table_lock = {.mem = OS_task_table_mut_mem};
static OS_impl_objtype_lock_t OS_queue_table_lock = {.mem = OS_queue_table_mut_mem};
Expand All @@ -70,6 +71,7 @@ static OS_impl_objtype_lock_t OS_timecb_table_lock = {.mem = OS_timecb_table_
static OS_impl_objtype_lock_t OS_module_table_lock = {.mem = OS_module_table_mut_mem};
static OS_impl_objtype_lock_t OS_filesys_table_lock = {.mem = OS_filesys_table_mut_mem};
static OS_impl_objtype_lock_t OS_console_table_lock = {.mem = OS_console_table_mut_mem};
static OS_impl_objtype_lock_t OS_condvar_table_lock = {.mem = OS_condvar_table_mut_mem};

OS_impl_objtype_lock_t *const OS_impl_objtype_lock_table[OS_OBJECT_TYPE_USER] = {
[OS_OBJECT_TYPE_UNDEFINED] = NULL,
Expand All @@ -84,7 +86,8 @@ OS_impl_objtype_lock_t *const OS_impl_objtype_lock_table[OS_OBJECT_TYPE_USER] =
[OS_OBJECT_TYPE_OS_TIMECB] = &OS_timecb_table_lock,
[OS_OBJECT_TYPE_OS_MODULE] = &OS_module_table_lock,
[OS_OBJECT_TYPE_OS_FILESYS] = &OS_filesys_table_lock,
[OS_OBJECT_TYPE_OS_CONSOLE] = &OS_console_table_lock};
[OS_OBJECT_TYPE_OS_CONSOLE] = &OS_console_table_lock,
[OS_OBJECT_TYPE_OS_CONDVAR] = &OS_condvar_table_lock};

/*----------------------------------------------------------------
*
Expand All @@ -100,9 +103,12 @@ void OS_Lock_Global_Impl(osal_objtype_t idtype)

impl = OS_impl_objtype_lock_table[idtype];

if (semTake(impl->vxid, WAIT_FOREVER) != OK)
if (impl != NULL)
{
OS_DEBUG("semTake() - vxWorks errno %d\n", errno);
if (semTake(impl->vxid, WAIT_FOREVER) != OK)
{
OS_DEBUG("semTake() - vxWorks errno %d\n", errno);
}
}

} /* end OS_Lock_Global_Impl */
Expand All @@ -121,9 +127,12 @@ void OS_Unlock_Global_Impl(osal_objtype_t idtype)

impl = OS_impl_objtype_lock_table[idtype];

if (semGive(impl->vxid) != OK)
if (impl != NULL)
{
OS_DEBUG("semGive() - vxWorks errno %d\n", errno);
if (semGive(impl->vxid) != OK)
{
OS_DEBUG("semGive() - vxWorks errno %d\n", errno);
}
}

} /* end OS_Unlock_Global_Impl */
Expand Down
12 changes: 10 additions & 2 deletions src/unit-test-coverage/vxworks/src/coveragetest-idmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ void Test_OS_Lock_Global_Impl(void)

UT_IdMapTest_SetImplTableMutex(OS_OBJECT_TYPE_OS_TASK, &TestGlobalSem);
OS_Lock_Global_Impl(OS_OBJECT_TYPE_OS_TASK);
UtAssert_True(UT_GetStubCount(UT_KEY(OCS_semTake)) == 1, "semTake() called");
UtAssert_STUB_COUNT(OCS_semTake, 1);

/* The "undefined" type should not have a lock instantiated */
OS_Lock_Global_Impl(OS_OBJECT_TYPE_UNDEFINED);
UtAssert_STUB_COUNT(OCS_semTake, 1);

UT_SetDefaultReturnValue(UT_KEY(OCS_semTake), -1);
OS_Lock_Global_Impl(OS_OBJECT_TYPE_OS_TASK); /* for coverage of error path */
Expand All @@ -57,7 +61,11 @@ void Test_OS_Unlock_Global_Impl(void)

UT_IdMapTest_SetImplTableMutex(OS_OBJECT_TYPE_OS_TASK, &TestGlobalSem);
OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TASK);
UtAssert_True(UT_GetStubCount(UT_KEY(OCS_semGive)) == 1, "semTake() called");
UtAssert_STUB_COUNT(OCS_semGive, 1);

/* The "undefined" type should not have a lock instantiated */
OS_Unlock_Global_Impl(OS_OBJECT_TYPE_UNDEFINED);
UtAssert_STUB_COUNT(OCS_semGive, 1);

UT_SetDefaultReturnValue(UT_KEY(OCS_semGive), -1);
OS_Unlock_Global_Impl(OS_OBJECT_TYPE_OS_TASK); /* for coverage of error path */
Expand Down

0 comments on commit 7bd5036

Please sign in to comment.