From 42135c1a22d0d5f902eac2da027ed5c43e4ae1c7 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Sat, 25 Jan 2025 10:26:19 +0000 Subject: [PATCH 1/3] 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: FreeRTOS/FreeRTOS-Kernel#1217 - Forum thread: freertos.org/t/22287 Signed-off-by: Gaurav Aggarwal --- .../GCC/Posix/utils/wait_for_event.c | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c b/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c index bf744e27f37..443bb790923 100644 --- a/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c +++ b/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c @@ -35,6 +35,7 @@ struct event { pthread_mutex_t mutex; + pthread_mutexattr_t mutexattr; pthread_cond_t cond; bool event_triggered; }; @@ -46,7 +47,9 @@ struct event * event_create( void ) if( ev != NULL ) { ev->event_triggered = false; - pthread_mutex_init( &ev->mutex, NULL ); + pthread_mutexattr_init( &ev->mutexattr ); + pthread_mutexattr_setrobust( &ev->mutexattr, PTHREAD_MUTEX_ROBUST ); + pthread_mutex_init( &ev->mutex, &ev->mutexattr ); pthread_cond_init( &ev->cond, NULL ); } @@ -56,13 +59,18 @@ struct event * event_create( void ) 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 ) + { + /* If the thread owning the mutex died, make the mutex consistent. */ + pthread_mutex_consistent( &ev->mutex ); + } while( ev->event_triggered == false ) { @@ -82,7 +90,11 @@ 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 ) + { + /* If the thread owning the mutex died, make the mutex consistent. */ + pthread_mutex_consistent( &ev->mutex ); + } while( ( ev->event_triggered == false ) && ( ret == 0 ) ) { @@ -101,7 +113,11 @@ bool event_wait_timed( struct event * ev, void event_signal( struct event * ev ) { - pthread_mutex_lock( &ev->mutex ); + if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD ) + { + /* If the thread owning the mutex died, make the mutex consistent. */ + pthread_mutex_consistent( &ev->mutex ); + } ev->event_triggered = true; pthread_cond_signal( &ev->cond ); pthread_mutex_unlock( &ev->mutex ); From 2e38eb5423e1fa9ccb8cb7704d9f7999691f2d2c Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Sat, 25 Jan 2025 11:12:42 +0000 Subject: [PATCH 2/3] Address warnings introduced in PR #1223 Signed-off-by: Gaurav Aggarwal --- portable/ThirdParty/GCC/Posix/port.c | 65 +++++++++++++++++++--------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/port.c b/portable/ThirdParty/GCC/Posix/port.c index c4eacb2ba61..4f7d8b609b0 100644 --- a/portable/ThirdParty/GCC/Posix/port.c +++ b/portable/ThirdParty/GCC/Posix/port.c @@ -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 ); @@ -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 ); } @@ -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 ); @@ -369,7 +390,7 @@ void vPortYield( void ) void vPortDisableInterrupts( void ) { - if( prvIsFreeRTOSThread( pthread_self() ) == pdTRUE ) + if( prvIsFreeRTOSThread() == pdTRUE ) { pthread_sigmask(SIG_BLOCK, &xAllSignals, NULL); } @@ -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 ); } } /*-----------------------------------------------------------*/ @@ -417,9 +438,9 @@ static void * prvTimerTickHandler( void * arg ) { ( void ) arg; - prvMarkAsFreeRTOSThread( pthread_self() ); + prvMarkAsFreeRTOSThread(); - prvPortSetCurrentThreadName("Scheduler timer"); + prvPortSetCurrentThreadName( "Scheduler timer" ); while( xTimerTickThreadShouldRun ) { @@ -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; @@ -473,7 +494,9 @@ static void vPortSystemTickHandler( int sig ) } uxCriticalNesting--; - } else { + } + else + { fprintf( stderr, "vPortSystemTickHandler called from non-FreeRTOS thread\n" ); } } @@ -508,7 +531,7 @@ static void * prvWaitForStart( void * pvParams ) { Thread_t * pxThread = pvParams; - prvMarkAsFreeRTOSThread( pthread_self() ); + prvMarkAsFreeRTOSThread(); prvSuspendSelf( pxThread ); From 92be5d47eea5dd73369c0f35f8022e81726fbd55 Mon Sep 17 00:00:00 2001 From: Gaurav Aggarwal Date: Sat, 25 Jan 2025 11:23:07 +0000 Subject: [PATCH 3/3] Mutex robust APIs are not available on Mac Signed-off-by: Gaurav Aggarwal --- .../GCC/Posix/utils/wait_for_event.c | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c b/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c index 443bb790923..55fd7bbfc93 100644 --- a/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c +++ b/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c @@ -39,6 +39,7 @@ struct event pthread_cond_t cond; bool event_triggered; }; +/*-----------------------------------------------------------*/ struct event * event_create( void ) { @@ -48,13 +49,16 @@ struct event * event_create( void ) { ev->event_triggered = false; pthread_mutexattr_init( &ev->mutexattr ); - pthread_mutexattr_setrobust( &ev->mutexattr, PTHREAD_MUTEX_ROBUST ); + #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 ) { @@ -63,13 +67,16 @@ void event_delete( struct event * ev ) pthread_cond_destroy( &ev->cond ); free( ev ); } +/*-----------------------------------------------------------*/ bool event_wait( struct event * ev ) { if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD ) { - /* If the thread owning the mutex died, make the mutex consistent. */ - pthread_mutex_consistent( &ev->mutex ); + #ifndef __APPLE__ + /* If the thread owning the mutex died, make the mutex consistent. */ + pthread_mutex_consistent( &ev->mutex ); + #endif } while( ev->event_triggered == false ) @@ -81,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 ) { @@ -92,8 +101,10 @@ bool event_wait_timed( struct event * ev, ts.tv_nsec += ( ( ms % 1000 ) * 1000000 ); if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD ) { - /* If the thread owning the mutex died, make the mutex consistent. */ - pthread_mutex_consistent( &ev->mutex ); + #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 ) ) @@ -110,15 +121,19 @@ bool event_wait_timed( struct event * ev, pthread_mutex_unlock( &ev->mutex ); return true; } +/*-----------------------------------------------------------*/ void event_signal( struct event * ev ) { if( pthread_mutex_lock( &ev->mutex ) == EOWNERDEAD ) { - /* If the thread owning the mutex died, make the mutex consistent. */ - pthread_mutex_consistent( &ev->mutex ); + #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 ); } +/*-----------------------------------------------------------*/