sergiotarxz/nuttx
sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix an assertion noted by Jussi Kivilinna.

Browse Source 
This was a consequence of the recent robust mutex changes.  If robust mutexes are selected, then each mutex that a thread takes is retained in a list in threads TCB.  If the thread exits and that list is not empty, then we know that the thread exitted while holding mutexes.  And, in that case, each will be marked as inconsistent and the any waiter for the thread is awakened.

For the case of pthread_mutex_trywait(), the mutex was not being added to the list!  while not usually a fatal error, this was caught by an assertion when pthread_mutex_unlock() was called:  It tried to remove the mutex from the TCB list and it was not there when, of course, it shoule be.

The fix was to add pthread_mutex_trytake() which does sem_trywait() and if successful, does correctly add the mutext to the TCB list.  This should eliminated the assertion.
This commit is contained in:
Gregory Nutt 2017-03-29 07:50:40 -06:00
parent 28e74ec058
commit eb344d7260
3 changed files with 180 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
sched/pthread/pthread.h
View File

@ -112,11 +112,13 @@ int pthread_givesemaphore(sem_t *sem);
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
int pthread_mutex_take(FAR struct pthread_mutex_s *mutex, bool intr);
int pthread_mutex_trytake(FAR struct pthread_mutex_s *mutex);
int pthread_mutex_give(FAR struct pthread_mutex_s *mutex);
void pthread_mutex_inconsistent(FAR struct pthread_tcb_s *tcb);
#else
# define pthread_mutex_take(m,i) pthread_takesemaphore(&(m)->sem,(i))
# define pthread_mutex_give(m) pthread_givesemaphore(&(m)->sem)
# define pthread_mutex_take(m,i) pthread_takesemaphore(&(m)->sem,(i))
# define pthread_mutex_trytake(m) sem_trywait(&(m)->sem)
# define pthread_mutex_give(m) pthread_givesemaphore(&(m)->sem)
#endif
#ifdef CONFIG_PTHREAD_MUTEX_TYPES

145
sched/pthread/pthread_mutex.c
View File

@ -50,6 +50,39 @@
#include "sched/sched.h"
#include "pthread/pthread.h"
/****************************************************************************
* Private Functions
****************************************************************************/
/****************************************************************************
* Name: pthread_mutex_add
*
* Description:
* Add the mutex to the list of mutexes held by this thread.
*
* Parameters:
* mutex - The mux to be locked
*
* Return Value:
* None
*
****************************************************************************/
static void pthread_mutex_add(FAR struct pthread_mutex_s *mutex)
{
FAR struct pthread_tcb_s *rtcb = (FAR struct pthread_tcb_s *)this_task();
irqstate_t flags;
DEBUGASSERT(mutex->flink == NULL);
/* Add the mutex to the list of mutexes held by this task */
flags = enter_critical_section();
mutex->flink = rtcb->mhead;
rtcb->mhead = mutex;
leave_critical_section(flags);
}
/****************************************************************************
* Public Functions
****************************************************************************/
@ -58,8 +91,8 @@
* Name: pthread_mutex_take
*
* Description:
* Take the pthread_mutex and, if successful, add the mutex to the ist of
* mutexes held by this thread.
* Take the pthread_mutex, waiting if necessary. If successful, add the
* mutex to the list of mutexes held by this thread.
*
* Parameters:
* mutex - The mux to be locked
@ -84,38 +117,104 @@ int pthread_mutex_take(FAR struct pthread_mutex_s *mutex, bool intr)
sched_lock();
/* Take semaphore underlying the mutex */
/* Error out if the mutex is already in an inconsistent state. */
ret = pthread_takesemaphore(&mutex->sem, intr);
if (ret == OK)
if ((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0)
{
DEBUGASSERT(mutex->flink == NULL);
ret = EOWNERDEAD;
}
else
{
/* Take semaphore underlying the mutex */
/* Check if the holder of the mutex has terminated. In that case,
* the state of the mutex is inconsistent and we return EOWNERDEAD.
*/
if ((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0)
ret = pthread_takesemaphore(&mutex->sem, intr);
if (ret < OK)
{
ret = EOWNERDEAD;
ret = get_errno();
}
/* Add the mutex to the list of mutexes held by this task */
else
{
FAR struct pthread_tcb_s *rtcb = (FAR struct pthread_tcb_s *)this_task();
irqstate_t flags;
/* Check if the holder of the mutex has terminated without
* releasing. In that case, the state of the mutex is
* inconsistent and we return EOWNERDEAD.
*/
flags = enter_critical_section();
mutex->flink = rtcb->mhead;
rtcb->mhead = mutex;
leave_critical_section(flags);
if ((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0)
{
ret = EOWNERDEAD;
}
/* Add the mutex to the list of mutexes held by this task */
else
{
pthread_mutex_add(mutex);
}
}
}
sched_unlock();
}
return ret;
}
/****************************************************************************
* Name: pthread_mutex_trytake
*
* Description:
* Try to take the pthread_mutex without waiting. If successful, add the
* mutex to the list of mutexes held by this thread.
*
* Parameters:
* mutex - The mux to be locked
* intr - false: ignore EINTR errors when locking; true tread EINTR as
* other errors by returning the errno value
*
* Return Value:
* 0 on success or an errno value on failure.
*
****************************************************************************/
int pthread_mutex_trytake(FAR struct pthread_mutex_s *mutex)
{
int ret = EINVAL;
/* Verify input parameters */
DEBUGASSERT(mutex != NULL);
if (mutex != NULL)
{
/* Make sure that no unexpected context switches occur */
sched_lock();
/* Error out if the mutex is already in an inconsistent state. */
if ((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0)
{
ret = EOWNERDEAD;
}
else
{
/* Try to take the semaphore underlying the mutex */
ret = sem_trywait(&mutex->sem);
if (ret < OK)
{
ret = get_errno();
}
else
{
/* Add the mutex to the list of mutexes held by this task */
pthread_mutex_add(mutex);
}
}
sched_unlock();
}
sched_unlock();
return ret;
}
@ -173,7 +272,7 @@ int pthread_mutex_give(FAR struct pthread_mutex_s *mutex)
mutex->flink = NULL;
leave_critical_section(flags);
/* Now release the underlying semaphore */
ret = pthread_givesemaphore(&mutex->sem);

113
sched/pthread/pthread_mutextrylock.c
View File

@ -102,7 +102,7 @@ int pthread_mutex_trylock(FAR pthread_mutex_t *mutex)
/* Try to get the semaphore. */
status = sem_trywait((FAR sem_t *)&mutex->sem);
status = pthread_mutex_trytake(mutex);
if (status == OK)
{
/* If we successfully obtained the semaphore, then indicate
@ -120,96 +120,91 @@ int pthread_mutex_trylock(FAR pthread_mutex_t *mutex)
ret = OK;
}
/* sem_trywait failed */
/* pthread_mutex_trytake failed. Did it fail because the semaphore
* was not avaialable?
*/
else
else if (status == EAGAIN)
{
/* Did it fail because the semaphore was not avaialable? */
int errcode = get_errno();
if (errcode == EAGAIN)
{
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
/* Check if recursive mutex was locked by the calling thread. */
/* Check if recursive mutex was locked by the calling thread. */
if (mutex->type == PTHREAD_MUTEX_RECURSIVE && mutex->pid == mypid)
if (mutex->type == PTHREAD_MUTEX_RECURSIVE && mutex->pid == mypid)
{
/* Increment the number of locks held and return successfully. */
if (mutex->nlocks < INT16_MAX)
{
/* Increment the number of locks held and return successfully. */
if (mutex->nlocks < INT16_MAX)
{
mutex->nlocks++;
ret = OK;
}
else
{
ret = EOVERFLOW;
}
mutex->nlocks++;
ret = OK;
}
else
{
ret = EOVERFLOW;
}
}
else
#endif
#ifndef CONFIG_PTHREAD_MUTEX_UNSAFE
/* The calling thread does not hold the semaphore. The correct
* behavior for the 'robust' mutex is to verify that the holder of
* the mutex is still valid. This is protection from the case
* where the holder of the mutex has exitted without unlocking it.
*/
/* The calling thread does not hold the semaphore. The correct
* behavior for the 'robust' mutex is to verify that the holder of
* the mutex is still valid. This is protection from the case
* where the holder of the mutex has exitted without unlocking it.
*/
#ifdef CONFIG_PTHREAD_MUTEX_BOTH
#ifdef CONFIG_PTHREAD_MUTEX_TYPES
/* Check if this NORMAL mutex is robust */
/* Check if this NORMAL mutex is robust */
if (mutex->pid > 0 &&
((mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 ||
mutex->type != PTHREAD_MUTEX_NORMAL) &&
sched_gettcb(mutex->pid) == NULL)
if (mutex->pid > 0 &&
((mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 ||
mutex->type != PTHREAD_MUTEX_NORMAL) &&
sched_gettcb(mutex->pid) == NULL)
#else /* CONFIG_PTHREAD_MUTEX_TYPES */
/* Check if this NORMAL mutex is robust */
/* Check if this NORMAL mutex is robust */
if (mutex->pid > 0 &&
(mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 &&
sched_gettcb(mutex->pid) == NULL)
if (mutex->pid > 0 &&
(mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 &&
sched_gettcb(mutex->pid) == NULL)
#endif /* CONFIG_PTHREAD_MUTEX_TYPES */
#else /* CONFIG_PTHREAD_MUTEX_ROBUST */
/* This mutex is always robust, whatever type it is. */
/* This mutex is always robust, whatever type it is. */
if (mutex->pid > 0 && sched_gettcb(mutex->pid) == NULL)
if (mutex->pid > 0 && sched_gettcb(mutex->pid) == NULL)
#endif
{
DEBUGASSERT(mutex->pid != 0); /* < 0: available, >0 owned, ==0 error */
DEBUGASSERT((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0);
{
DEBUGASSERT(mutex->pid != 0); /* < 0: available, >0 owned, ==0 error */
DEBUGASSERT((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0);
/* A thread holds the mutex, but there is no such thread.
* POSIX requires that the 'robust' mutex return EOWNERDEAD
* in this case. It is then the caller's responsibility to
* call pthread_mutx_consistent() fo fix the mutex.
*/
/* A thread holds the mutex, but there is no such thread.
* POSIX requires that the 'robust' mutex return EOWNERDEAD
* in this case. It is then the caller's responsibility to
* call pthread_mutx_consistent() fo fix the mutex.
*/
mutex->flags |= _PTHREAD_MFLAGS_INCONSISTENT;
ret = EOWNERDEAD;
}
/* The mutex is locked by another, active thread */
else
#endif /* CONFIG_PTHREAD_MUTEX_UNSAFE */
{
ret = EBUSY;
}
mutex->flags |= _PTHREAD_MFLAGS_INCONSISTENT;
ret = EOWNERDEAD;
}
/* Some other, unhandled error occurred */
/* The mutex is locked by another, active thread */
else
#endif /* CONFIG_PTHREAD_MUTEX_UNSAFE */
{
ret = errcode;
ret = EBUSY;
}
}
/* Some other, unhandled error occurred */
else
{
ret = status;
}
sched_unlock();
}