Skip to content

Commit

Permalink
sched/pthread/pthread_condclockwait.c: follow POSIX.
Browse files Browse the repository at this point in the history 
pthread_condclockwait() can not distinguish between interrupt and timeout,
which cause these API not follow POSIX:
	pthread_rwlock_timedrdlock()
	pthread_rwlock_timedwrlock()
	pthread_condtimedwait()
POSIX:
	Upon return from the signal handler the thread resumes waiting for
	the condition variable as if it wasnot interrupted
	These functions shall not return an error code of [EINTR].

Replacing nxsem_wait() with nxsem_clockwait_uninterruptible() can solve it.

Signed-off-by: jihandong <jihandong@xiaomi.com>
  • Loading branch information
@jihandong
jihandong committed Apr 11, 2022
1 parent 8d66dbc commit 2ad8cae
Showing 1 changed file with 49 additions and 194 deletions.
243 changes: 49 additions & 194 deletions sched/pthread/pthread_condclockwait.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,86 +44,6 @@
#include "clock/clock.h"
#include "signal/signal.h"

/****************************************************************************
* Private Functions
****************************************************************************/

/****************************************************************************
* Name: pthread_condtimedout
*
* Description:
* This function is called if the timeout elapses before
* the condition is signaled.
*
* Input Parameters:
* arg - the argument
*
* Returned Value:
* None
*
* Assumptions:
*
****************************************************************************/

static void pthread_condtimedout(wdparm_t arg)
{
pid_t pid = arg;

#ifdef HAVE_GROUP_MEMBERS
{
FAR struct tcb_s *tcb;
siginfo_t info;

/* The logic below if equivalent to nxsig_queue(), but uses
* nxsig_tcbdispatch() instead of nxsig_dispatch(). This avoids the
* group signal deliver logic and assures, instead, that the signal is
* delivered specifically to this thread that is known to be waiting on
* the signal.
*/

/* Get the waiting TCB. nxsched_get_tcb() might return NULL if the
* task has exited for some reason.
*/

tcb = nxsched_get_tcb(pid);
if (tcb)
{
/* Create the siginfo structure */

info.si_signo = SIGCONDTIMEDOUT;
info.si_code = SI_QUEUE;
info.si_errno = ETIMEDOUT;
info.si_value.sival_ptr = NULL;
#ifdef CONFIG_SCHED_HAVE_PARENT
info.si_pid = pid;
info.si_status = OK;
#endif

/* Process the receipt of the signal. The scheduler is not locked
* as is normally the case when this function is called because we
* are in a watchdog timer interrupt handler.
*/

nxsig_tcbdispatch(tcb, &info);
}
}
#else /* HAVE_GROUP_MEMBERS */
{
/* Things are a little easier if there are not group members. We can
* just use nxsig_queue().
*/

union sigval value;

/* Send the specified signal to the specified task. */

value.sival_ptr = NULL;
nxsig_queue(pid, SIGCONDTIMEDOUT, value);
}

#endif /* HAVE_GROUP_MEMBERS */
}

/****************************************************************************
* Public Functions
****************************************************************************/
Expand Down Expand Up @@ -153,9 +73,7 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
clockid_t clockid,
FAR const struct timespec *abstime)
{
FAR struct tcb_s *rtcb = this_task();
irqstate_t flags;
sclock_t ticks;
int mypid = getpid();
int ret = OK;
int status;
Expand Down Expand Up @@ -202,134 +120,71 @@ int pthread_cond_clockwait(FAR pthread_cond_t *cond,
sched_lock();
flags = enter_critical_section();

/* Convert the timespec to clock ticks. We must disable pre-
* emption here so that this time stays valid until the wait
* begins.
*/
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
uint8_t mflags;
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
uint8_t type;
int16_t nlocks;
#endif
/* Give up the mutex */

ret = clock_abstime2ticks(clockid, abstime, &ticks);
if (ret)
mutex->pid = INVALID_PROCESS_ID;
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
mflags = mutex->flags;
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
type = mutex->type;
nlocks = mutex->nlocks;
#endif
ret = pthread_mutex_give(mutex);
if (ret != 0)
{
/* Restore interrupts (pre-emption will be enabled when
* we fall through the if/then/else)
/* Restore interrupts (pre-emption will be enabled
* when we fall through the if/then/else)
*/

leave_critical_section(flags);
}
else
{
/* Check the absolute time to wait. If it is now or in the
* past, then just return with the timedout condition.
*/
/* Take the condition semaphore. */

if (ticks <= 0)
status = nxsem_clockwait_uninterruptible(
(FAR sem_t *)&cond->sem, clockid, abstime);
if (status < 0)
{
/* Restore interrupts and indicate that we have already
* timed out. (pre-emption will be enabled when we fall
* through the if/then/else
*/

leave_critical_section(flags);
ret = ETIMEDOUT;
ret = -status;
}
else
{
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
uint8_t mflags;
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
uint8_t type;
int16_t nlocks;
#endif
/* Give up the mutex */

mutex->pid = INVALID_PROCESS_ID;
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
mflags = mutex->flags;
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
type = mutex->type;
nlocks = mutex->nlocks;
#endif
ret = pthread_mutex_give(mutex);
if (ret != 0)
{
/* Restore interrupts (pre-emption will be enabled
* when we fall through the if/then/else)
*/

leave_critical_section(flags);
}
else
{
/* Start the watchdog */

wd_start(&rtcb->waitdog, ticks,
pthread_condtimedout, mypid);

/* Take the condition semaphore. Do not restore
* interrupts until we return from the wait. This is
* necessary to make sure that the watchdog timer and
* the condition wait are started atomically.
*/

status = nxsem_wait((FAR sem_t *)&cond->sem);

/* We no longer need the watchdog */

wd_cancel(&rtcb->waitdog);

/* Did we get the condition semaphore. */

if (status < 0)
{
/* NO.. Handle the special case where the semaphore
* wait was awakened by the receipt of a signal --
* presumably the signal posted by
* pthread_condtimedout().
*/

if (status == -EINTR)
{
swarn("WARNING: Timedout!\n");
ret = ETIMEDOUT;
}
else
{
ret = status;
}
}

/* The interrupts stay disabled until after we sample
* the errno. This is because when debug is enabled
* and the console is used for debug output, then the
* errno can be altered by interrupt handling! (bad)
*/

leave_critical_section(flags);
}

/* Reacquire the mutex (retaining the ret). */

sinfo("Re-locking...\n");

status = pthread_mutex_take(mutex, NULL, false);
if (status == OK)
{
mutex->pid = mypid;
/* The interrupts stay disabled until after we sample
* the errno. This is because when debug is enabled
* and the console is used for debug output, then the
* errno can be altered by interrupt handling! (bad)
*/

leave_critical_section(flags);
}

/* Reacquire the mutex (retaining the ret). */

sinfo("Re-locking...\n");

status = pthread_mutex_take(mutex, NULL, false);
if (status == OK)
{
mutex->pid = mypid;
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
mutex->flags = mflags;
mutex->flags = mflags;
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
mutex->type = type;
mutex->nlocks = nlocks;
mutex->type = type;
mutex->nlocks = nlocks;
#endif
}
else if (ret == 0)
{
ret = status;
}
}
}
else if (ret == 0)
{
ret = status;
}

/* Re-enable pre-emption (It is expected that interrupts
Expand Down

0 comments on commit 2ad8cae

Please sign in to comment.