Second parameter of pthread_mutex_timedlock should be absolute time not timeout

[jonathan-brandenburg-metecs]

In src/os/posix/src/os-impl-binsem.c, there are three calls to pthread_mutex_timedlock, each time with &OS_POSIX_BINSEM_MAX_WAIT as the second parameter where OS_POSIX_BINSEM_MAX_WAIT has a value of 2 seconds. because the second parameter of pthread_mutex_timedlock should be an absolute time and is not a timeout, if the mutex is not immediately available, pthread_mutex_timedlock will return with ETIMEDOUT resulting in the OS_BinSemFlush_Impl, OS_GenericBinSemTake_Impl, or OS_BinSemGive_Impl call returning OS_SEM_FAILURE.

Here's a code snippet of the "as is":
if ( pthread_mutex_timedlock(&sem->id, &OS_POSIX_BINSEM_MAX_WAIT) != 0 )
{
return(OS_SEM_FAILURE);
}

It should look something like:

struct timespec timeoutTime;
clock_gettime(CLOCK_REALTIME, &timeoutTime);
timeoutTime.tv_sec += OS_POSIX_BINSEM_MAX_WAIT.tv_sec;
timeoutTime.tv_nsec += OS_POSIX_BINSEM_MAX_WAIT.tv_nsec;
if ( pthread_mutex_timedlock(&sem->id, &timeoutTime) != 0 )
{
return(OS_SEM_FAILURE);
}

Reproducing this issue is tricky as the cFS application has to run long enough to have a mutex not immediately available for the lock. The behavior I'm seeing in my installation is the background task terminates and Ctrl-C can no longer be used to terminate the application. I'm running in a Centos 7 Virtual Box machine.

Jonathan C. Brandenburg
METECS
[email protected]

[jphickey]

Thanks for catching this! I somehow missed that the timeout here was absolute and not relative. Kind of annoying that POSIX isn't consistent here.

[jonathan-brandenburg-metecs]

Glad to help! It is a bit annoying. I read the documentation for that method 100 times and kept seeing "absolute" but it just didn't compute because that's not at all common. Thank you!