Skip to content

Commit

Permalink
Mark mutex as robust to prevent deadlocks (#1233)
Browse files Browse the repository at this point in the history 
Mark mutex as robust to prevent deadlocks

Prevent application hangs that occur when a thread dies while holding a
mutex, particularly during vTaskEndScheduler or exit calls. This is
achieved by setting the PTHREAD_MUTEX_ROBUST attribute on the mutex.

Fixes:
- GitHub issue: #1217
- Forum thread: freertos.org/t/22287

Signed-off-by: Gaurav Aggarwal <[email protected]>
  • Loading branch information
@aggarg
aggarg authored Jan 29, 2025
1 parent f94bc89 commit ad4e723
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 25 deletions.
65 changes: 44 additions & 21 deletions portable/ThirdParty/GCC/Posix/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,20 @@ static void prvResumeThread( Thread_t * xThreadId );
static void vPortSystemTickHandler( int sig );
static void vPortStartFirstTask( void );
static void prvPortYieldFromISR( void );
static void prvThreadKeyDestructor( void * pvData );
static void prvInitThreadKey( void );
static void prvMarkAsFreeRTOSThread( void );
static BaseType_t prvIsFreeRTOSThread( void );
static void prvDestroyThreadKey( void );
/*-----------------------------------------------------------*/

void prvThreadKeyDestructor( void * data )
static void prvThreadKeyDestructor( void * pvData )
{
free( data );
free( pvData );
}
/*-----------------------------------------------------------*/

static void prvInitThreadKey()
static void prvInitThreadKey( void )
{
pthread_mutex_lock( &xThreadMutex );

Expand All @@ -137,24 +143,39 @@ static void prvInitThreadKey()

pthread_mutex_unlock( &xThreadMutex );
}
/*-----------------------------------------------------------*/

static void prvMarkAsFreeRTOSThread( pthread_t thread )
static void prvMarkAsFreeRTOSThread( void )
{
uint8_t * pucThreadData = NULL;

prvInitThreadKey();
uint8_t * thread_data = malloc( 1 );
configASSERT( thread_data != NULL );
*thread_data = 1;
pthread_setspecific( xThreadKey, thread_data );

pucThreadData = malloc( 1 );
configASSERT( pucThreadData != NULL );

*pucThreadData = 1;

pthread_setspecific( xThreadKey, pucThreadData );
}
/*-----------------------------------------------------------*/

static BaseType_t prvIsFreeRTOSThread( pthread_t thread )
static BaseType_t prvIsFreeRTOSThread( void )
{
uint8_t * thread_data = ( uint8_t * ) pthread_getspecific( xThreadKey );
uint8_t * pucThreadData = NULL;
BaseType_t xRet = pdFALSE;

return thread_data != NULL && *thread_data == 1;
pucThreadData = ( uint8_t * ) pthread_getspecific( xThreadKey );
if( ( pucThreadData != NULL ) && ( *pucThreadData == 1 ) )
{
xRet = pdTRUE;
}

return xRet;
}
/*-----------------------------------------------------------*/

static void prvDestroyThreadKey()
static void prvDestroyThreadKey( void )
{
pthread_key_delete( xThreadKey );
}
Expand Down Expand Up @@ -309,7 +330,7 @@ void vPortEndScheduler( void )
( void ) pthread_kill( hMainThread, SIG_RESUME );

/* Waiting to be deleted here. */
if( prvIsFreeRTOSThread( pthread_self() ) == pdTRUE )
if( prvIsFreeRTOSThread() == pdTRUE )
{
pxCurrentThread = prvGetThreadFromTask( xTaskGetCurrentTaskHandle() );
event_wait( pxCurrentThread->ev );
Expand Down Expand Up @@ -369,7 +390,7 @@ void vPortYield( void )

void vPortDisableInterrupts( void )
{
if( prvIsFreeRTOSThread( pthread_self() ) == pdTRUE )
if( prvIsFreeRTOSThread() == pdTRUE )
{
pthread_sigmask(SIG_BLOCK, &xAllSignals, NULL);
}
Expand All @@ -378,9 +399,9 @@ void vPortDisableInterrupts( void )

void vPortEnableInterrupts( void )
{
if( prvIsFreeRTOSThread( pthread_self() ) == pdTRUE )
if( prvIsFreeRTOSThread() == pdTRUE )
{
pthread_sigmask(SIG_UNBLOCK, &xAllSignals, NULL);
pthread_sigmask( SIG_UNBLOCK, &xAllSignals, NULL );
}
}
/*-----------------------------------------------------------*/
Expand Down Expand Up @@ -417,9 +438,9 @@ static void * prvTimerTickHandler( void * arg )
{
( void ) arg;

prvMarkAsFreeRTOSThread( pthread_self() );
prvMarkAsFreeRTOSThread();

prvPortSetCurrentThreadName("Scheduler timer");
prvPortSetCurrentThreadName( "Scheduler timer" );

while( xTimerTickThreadShouldRun )
{
Expand Down Expand Up @@ -451,7 +472,7 @@ void prvSetupTimerInterrupt( void )

static void vPortSystemTickHandler( int sig )
{
if( prvIsFreeRTOSThread( pthread_self() ) == pdTRUE )
if( prvIsFreeRTOSThread() == pdTRUE )
{
Thread_t * pxThreadToSuspend;
Thread_t * pxThreadToResume;
Expand All @@ -473,7 +494,9 @@ static void vPortSystemTickHandler( int sig )
}

uxCriticalNesting--;
} else {
}
else
{
fprintf( stderr, "vPortSystemTickHandler called from non-FreeRTOS thread\n" );
}
}
Expand Down Expand Up @@ -508,7 +531,7 @@ static void * prvWaitForStart( void * pvParams )
{
Thread_t * pxThread = pvParams;

prvMarkAsFreeRTOSThread( pthread_self() );
prvMarkAsFreeRTOSThread();

prvSuspendSelf( pxThread );

Expand Down
39 changes: 35 additions & 4 deletions portable/ThirdParty/GCC/Posix/utils/wait_for_event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@
struct event
{
pthread_mutex_t mutex;
pthread_mutexattr_t mutexattr;
pthread_cond_t cond;
bool event_triggered;
};
/*-----------------------------------------------------------*/

struct event * event_create( void )
{
Expand All @@ -46,23 +48,36 @@ struct event * event_create( void )
if( ev != NULL )
{
ev->event_triggered = false;
pthread_mutex_init( &ev->mutex, NULL );
pthread_mutexattr_init( &ev->mutexattr );
#ifndef __APPLE__
pthread_mutexattr_setrobust( &ev->mutexattr, PTHREAD_MUTEX_ROBUST );
#endif
pthread_mutex_init( &ev->mutex, &ev->mutexattr );
pthread_cond_init( &ev->cond, NULL );
}

return ev;
}
/*-----------------------------------------------------------*/

void event_delete( struct event * ev )
{
pthread_mutex_destroy( &ev->mutex );
pthread_mutexattr_destroy( &ev->mutexattr );
pthread_cond_destroy( &ev->cond );
free( ev );
}
/*-----------------------------------------------------------*/

bool event_wait( struct event * ev )
{
pthread_mutex_lock( &ev->mutex );
if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD )
{
#ifndef __APPLE__
/* If the thread owning the mutex died, make the mutex consistent. */
pthread_mutex_consistent( &ev->mutex );
#endif
}

while( ev->event_triggered == false )
{
Expand All @@ -73,6 +88,8 @@ bool event_wait( struct event * ev )
pthread_mutex_unlock( &ev->mutex );
return true;
}
/*-----------------------------------------------------------*/

bool event_wait_timed( struct event * ev,
time_t ms )
{
Expand All @@ -82,7 +99,13 @@ bool event_wait_timed( struct event * ev,
clock_gettime( CLOCK_REALTIME, &ts );
ts.tv_sec += ms / 1000;
ts.tv_nsec += ( ( ms % 1000 ) * 1000000 );
pthread_mutex_lock( &ev->mutex );
if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD )
{
#ifndef __APPLE__
/* If the thread owning the mutex died, make the mutex consistent. */
pthread_mutex_consistent( &ev->mutex );
#endif
}

while( ( ev->event_triggered == false ) && ( ret == 0 ) )
{
Expand All @@ -98,11 +121,19 @@ bool event_wait_timed( struct event * ev,
pthread_mutex_unlock( &ev->mutex );
return true;
}
/*-----------------------------------------------------------*/

void event_signal( struct event * ev )
{
pthread_mutex_lock( &ev->mutex );
if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD )
{
#ifndef __APPLE__
/* If the thread owning the mutex died, make the mutex consistent. */
pthread_mutex_consistent( &ev->mutex );
#endif
}
ev->event_triggered = true;
pthread_cond_signal( &ev->cond );
pthread_mutex_unlock( &ev->mutex );
}
/*-----------------------------------------------------------*/

0 comments on commit ad4e723

Please sign in to comment.