From 1ac4a85ebf0cd0e788c2d5374c635087c33de0bf Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Thu, 25 Feb 2021 19:08:52 +0100 Subject: score: Fix thread initialization Close the thread object if a thread create extension fails. Also call the delete extension to avoid resource leaks in early extensions if a late extension fails. Close #4270. --- cpukit/include/rtems/posix/pthreadimpl.h | 7 -- cpukit/include/rtems/score/threadimpl.h | 22 +++- cpukit/posix/src/pthreadcreate.c | 39 ++++---- cpukit/rtems/src/taskconstruct.c | 23 ++--- cpukit/score/src/mpci.c | 7 +- cpukit/score/src/threadcreateidle.c | 7 +- cpukit/score/src/threadinitialize.c | 167 +++++++++++++++++++++---------- cpukit/score/src/threadrestart.c | 51 +--------- 8 files changed, 172 insertions(+), 151 deletions(-) (limited to 'cpukit') diff --git a/cpukit/include/rtems/posix/pthreadimpl.h b/cpukit/include/rtems/posix/pthreadimpl.h index 74e46e4d92..52d462ab6f 100644 --- a/cpukit/include/rtems/posix/pthreadimpl.h +++ b/cpukit/include/rtems/posix/pthreadimpl.h @@ -107,13 +107,6 @@ RTEMS_INLINE_ROUTINE Thread_Control *_POSIX_Threads_Allocate(void) _Objects_Allocate_unprotected( &_POSIX_Threads_Information.Objects ); } -RTEMS_INLINE_ROUTINE void _POSIX_Threads_Free ( - Thread_Control *the_pthread -) -{ - _Objects_Free( &_POSIX_Threads_Information.Objects, &the_pthread->Object ); -} - /** @} */ #ifdef __cplusplus diff --git a/cpukit/include/rtems/score/threadimpl.h b/cpukit/include/rtems/score/threadimpl.h index 0c7df47f36..1e7d58609f 100644 --- a/cpukit/include/rtems/score/threadimpl.h +++ b/cpukit/include/rtems/score/threadimpl.h @@ -201,15 +201,31 @@ typedef struct { * @param the_thread The thread to initialize. * @param config The configuration of the thread to initialize. * - * @retval true The thread initialization was successful. - * @retval false The thread initialization failed. + * @retval STATUS_SUCCESSFUL The thread initialization was successful. + * + * @retval STATUS_UNSATISFIED The thread initialization failed. */ -bool _Thread_Initialize( +Status_Control _Thread_Initialize( Thread_Information *information, Thread_Control *the_thread, const Thread_Configuration *config ); +/** + * @brief Frees the thread. + * + * This routine invokes the thread delete extensions and frees all resources + * associated with the thread. Afterwards the thread object is closed. + * + * @param[in, out] information is the thread information. + * + * @param[in, out] the_thread is the thread to free. + */ +void _Thread_Free( + Thread_Information *information, + Thread_Control *the_thread +); + /** * @brief Starts the specified thread. * diff --git a/cpukit/posix/src/pthreadcreate.c b/cpukit/posix/src/pthreadcreate.c index 33c5f8d03a..55ba73c8b4 100644 --- a/cpukit/posix/src/pthreadcreate.c +++ b/cpukit/posix/src/pthreadcreate.c @@ -26,6 +26,7 @@ #include #include +#include #include #if defined(RTEMS_POSIX_API) #include @@ -72,7 +73,8 @@ int pthread_create( int normal_prio; bool valid; Thread_Configuration config; - bool status; + Status_Control status; + bool ok; Thread_Control *the_thread; Thread_Control *executing; int schedpolicy = SCHED_RR; @@ -224,22 +226,23 @@ int pthread_create( config.stack_free = _Stack_Free_nothing; } - status = ( config.stack_area != NULL ); + if ( config.stack_area == NULL ) { + _Objects_Free( &_POSIX_Threads_Information.Objects, &the_thread->Object ); + _Objects_Allocator_unlock(); + return EAGAIN; + } /* * Initialize the core thread for this task. */ - if ( status ) { - status = _Thread_Initialize( - &_POSIX_Threads_Information, - the_thread, - &config - ); - } - if ( !status ) { - _POSIX_Threads_Free( the_thread ); + status = _Thread_Initialize( + &_POSIX_Threads_Information, + the_thread, + &config + ); + if ( status != STATUS_SUCCESSFUL ) { _Objects_Allocator_unlock(); - return EAGAIN; + return _POSIX_Get_error( status ); } if ( the_attr->detachstate == PTHREAD_CREATE_DETACHED ) { @@ -249,14 +252,14 @@ int pthread_create( the_thread->Life.state |= THREAD_LIFE_CHANGE_DEFERRED; _ISR_lock_ISR_disable( &lock_context ); - status = _Scheduler_Set_affinity( + ok = _Scheduler_Set_affinity( the_thread, the_attr->affinitysetsize, the_attr->affinityset ); _ISR_lock_ISR_enable( &lock_context ); - if ( !status ) { - _POSIX_Threads_Free( the_thread ); + if ( !ok ) { + _Thread_Free( &_POSIX_Threads_Information, the_thread ); _RTEMS_Unlock_allocator(); return EINVAL; } @@ -287,7 +290,7 @@ int pthread_create( * POSIX threads are allocated and started in one operation. */ _ISR_lock_ISR_disable( &lock_context ); - status = _Thread_Start( the_thread, &entry, &lock_context ); + ok = _Thread_Start( the_thread, &entry, &lock_context ); #if defined(RTEMS_DEBUG) /* @@ -296,8 +299,8 @@ int pthread_create( * NOTE: This can only happen if someone slips in and touches the * thread while we are creating it. */ - if ( !status ) { - _POSIX_Threads_Free( the_thread ); + if ( !ok ) { + _Thread_Free( &_POSIX_Threads_Information, the_thread ); _Objects_Allocator_unlock(); return EINVAL; } diff --git a/cpukit/rtems/src/taskconstruct.c b/cpukit/rtems/src/taskconstruct.c index 799554c417..397f6c2c89 100644 --- a/cpukit/rtems/src/taskconstruct.c +++ b/cpukit/rtems/src/taskconstruct.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -79,14 +80,6 @@ rtems_status_code rtems_task_construct( return _RTEMS_tasks_Create( config, id, _RTEMS_tasks_Prepare_user_stack ); } -static void _RTEMS_tasks_Free( Thread_Control *the_thread ) -{ - Thread_Information *information; - - information = _Thread_Get_objects_information( the_thread ); - _Objects_Free( &information->Objects, &the_thread->Object ); -} - rtems_status_code _RTEMS_tasks_Create( const rtems_task_config *config, rtems_id *id, @@ -190,7 +183,7 @@ rtems_status_code _RTEMS_tasks_Create( the_global_object = _Objects_MP_Allocate_global_object(); if ( the_global_object == NULL ) { - _RTEMS_tasks_Free( the_thread ); + _Objects_Free( &_RTEMS_tasks_Information.Objects, &the_thread->Object ); _Objects_Allocator_unlock(); return RTEMS_TOO_MANY; } @@ -204,17 +197,16 @@ rtems_status_code _RTEMS_tasks_Create( */ if ( status == RTEMS_SUCCESSFUL ) { - bool ok; + Status_Control score_status; - ok = _Thread_Initialize( + score_status = _Thread_Initialize( &_RTEMS_tasks_Information, the_thread, &thread_config ); - - if ( !ok ) { - status = RTEMS_UNSATISFIED; - } + status = _Status_Get( score_status ); + } else { + _Objects_Free( &_RTEMS_tasks_Information.Objects, &the_thread->Object ); } if ( status != RTEMS_SUCCESSFUL ) { @@ -222,7 +214,6 @@ rtems_status_code _RTEMS_tasks_Create( if ( is_global ) _Objects_MP_Free_global_object( the_global_object ); #endif - _RTEMS_tasks_Free( the_thread ); _Objects_Allocator_unlock(); return status; } diff --git a/cpukit/score/src/mpci.c b/cpukit/score/src/mpci.c index 91d0eb0214..cb306c9763 100644 --- a/cpukit/score/src/mpci.c +++ b/cpukit/score/src/mpci.c @@ -126,7 +126,7 @@ static void _MPCI_Create_server( void ) } }; Thread_Configuration config; - bool ok; + Status_Control status; ISR_lock_Context lock_context; @@ -152,13 +152,12 @@ static void _MPCI_Create_server( void ) + CPU_ALL_TASKS_ARE_FP * CONTEXT_FP_SIZE; config.stack_area = _MPCI_Receive_server_stack; - ok = _Thread_Initialize( + status = _Thread_Initialize( &_Thread_Information, _MPCI_Receive_server_tcb, &config ); - _Assert( ok ); - (void) ok; + _Assert_Unused_variable_equals( status, STATUS_SUCCESSFUL ); _ISR_lock_ISR_disable( &lock_context ); _Thread_Start( _MPCI_Receive_server_tcb, &entry, &lock_context ); diff --git a/cpukit/score/src/threadcreateidle.c b/cpukit/score/src/threadcreateidle.c index 04a103cf14..395dcc9c12 100644 --- a/cpukit/score/src/threadcreateidle.c +++ b/cpukit/score/src/threadcreateidle.c @@ -34,7 +34,7 @@ static void _Thread_Create_idle_for_CPU( Per_CPU_Control *cpu ) { Thread_Configuration config; Thread_Control *idle; - bool ok; + Status_Control status; memset( &config, 0, sizeof( config ) ); config.scheduler = _Scheduler_Get_by_CPU( cpu ); @@ -67,9 +67,8 @@ static void _Thread_Create_idle_for_CPU( Per_CPU_Control *cpu ) idle = _Thread_Internal_allocate(); _Assert( idle != NULL ); - ok = _Thread_Initialize( &_Thread_Information, idle, &config ); - _Assert( ok ); - (void) ok; + status = _Thread_Initialize( &_Thread_Information, idle, &config ); + _Assert_Unused_variable_equals( status, STATUS_SUCCESSFUL ); /* * WARNING!!! This is necessary to "kick" start the system and diff --git a/cpukit/score/src/threadinitialize.c b/cpukit/score/src/threadinitialize.c index 10eb0d7a3f..3a331ed269 100644 --- a/cpukit/score/src/threadinitialize.c +++ b/cpukit/score/src/threadinitialize.c @@ -29,14 +29,81 @@ #include #include -bool _Thread_Initialize( +void _Thread_Free( + Thread_Information *information, + Thread_Control *the_thread +) +{ +#if defined(RTEMS_SMP) + Scheduler_Node *scheduler_node; + size_t scheduler_index; +#endif + + _User_extensions_Thread_delete( the_thread ); + _User_extensions_Destroy_iterators( the_thread ); + _ISR_lock_Destroy( &the_thread->Keys.Lock ); + +#if defined(RTEMS_SMP) + scheduler_node = the_thread->Scheduler.nodes; + scheduler_index = 0; + + while ( scheduler_index < _Scheduler_Count ) { + _Scheduler_Node_destroy( + &_Scheduler_Table[ scheduler_index ], + scheduler_node + ); + scheduler_node = (Scheduler_Node *) + ( (uintptr_t) scheduler_node + _Scheduler_Node_size ); + ++scheduler_index; + } +#else + _Scheduler_Node_destroy( + _Thread_Scheduler_get_home( the_thread ), + _Thread_Scheduler_get_home_node( the_thread ) + ); +#endif + + _ISR_lock_Destroy( &the_thread->Timer.Lock ); + + /* + * The thread might have been FP. So deal with that. + */ +#if ( CPU_HARDWARE_FP == TRUE ) || ( CPU_SOFTWARE_FP == TRUE ) +#if ( CPU_USE_DEFERRED_FP_SWITCH == TRUE ) + if ( _Thread_Is_allocated_fp( the_thread ) ) + _Thread_Deallocate_fp(); +#endif +#endif + + _Freechain_Push( + &information->Thread_queue_heads.Free, + the_thread->Wait.spare_heads + ); + + /* + * Free the rest of the memory associated with this task + * and set the associated pointers to NULL for safety. + */ + ( *the_thread->Start.stack_free )( the_thread->Start.Initial_stack.area ); + +#if defined(RTEMS_SMP) + _ISR_lock_Destroy( &the_thread->Scheduler.Lock ); + _ISR_lock_Destroy( &the_thread->Wait.Lock.Default ); + _SMP_lock_Stats_destroy( &the_thread->Potpourri_stats ); +#endif + + _Thread_queue_Destroy( &the_thread->Join_queue ); + _Context_Destroy( the_thread, &the_thread->Registers ); + _Objects_Free( &information->Objects, &the_thread->Object ); +} + +static bool _Thread_Try_initialize( Thread_Information *information, Thread_Control *the_thread, const Thread_Configuration *config ) { uintptr_t tls_size; - bool extension_status; size_t i; char *stack_begin; char *stack_end; @@ -44,8 +111,8 @@ bool _Thread_Initialize( #if defined(RTEMS_SMP) Scheduler_Node *scheduler_node_for_index; const Scheduler_Control *scheduler_for_index; -#endif size_t scheduler_index; +#endif Per_CPU_Control *cpu = _Per_CPU_Get_by_index( 0 ); memset( @@ -61,29 +128,6 @@ bool _Thread_Initialize( (char *) the_thread + add_on->source_offset; } - /* Set everything to perform the error case clean up */ - the_thread->Start.stack_free = config->stack_free; - -#if defined(RTEMS_SMP) - if ( - !config->is_preemptible - && !_Scheduler_Is_non_preempt_mode_supported( config->scheduler ) - ) { - goto failed; - } -#endif - -#if defined(RTEMS_SMP) || CPU_ENABLE_ROBUST_THREAD_DISPATCH == TRUE - if ( - config->isr_level != 0 -#if CPU_ENABLE_ROBUST_THREAD_DISPATCH == FALSE - && _SMP_Need_inter_processor_interrupts() -#endif - ) { - goto failed; - } -#endif - stack_begin = config->stack_area; stack_end = stack_begin + config->stack_size; @@ -131,6 +175,7 @@ bool _Thread_Initialize( the_thread->Start.is_preemptible = config->is_preemptible; the_thread->Start.budget_algorithm = config->budget_algorithm; the_thread->Start.budget_callout = config->budget_callout; + the_thread->Start.stack_free = config->stack_free; _Thread_Timer_initialize( &the_thread->Timer, cpu ); @@ -202,7 +247,6 @@ bool _Thread_Initialize( the_thread, config->priority ); - scheduler_index = 1; #endif _Priority_Node_initialize( &the_thread->Real_priority, config->priority ); @@ -243,6 +287,33 @@ bool _Thread_Initialize( _Objects_Open_u32( &information->Objects, &the_thread->Object, config->name ); + /* + * We do following checks of simple error conditions after the thread is + * fully initialized to simplify the clean up in case of an error. With a + * fully initialized thread we can simply use _Thread_Free() and do not have + * to bother with partially initialized threads. + */ + +#if defined(RTEMS_SMP) + if ( + !config->is_preemptible + && !_Scheduler_Is_non_preempt_mode_supported( config->scheduler ) + ) { + return false; + } +#endif + +#if defined(RTEMS_SMP) || CPU_ENABLE_ROBUST_THREAD_DISPATCH == TRUE + if ( + config->isr_level != 0 +#if CPU_ENABLE_ROBUST_THREAD_DISPATCH == FALSE + && _SMP_Need_inter_processor_interrupts() +#endif + ) { + return false; + } +#endif + /* * We assume the Allocator Mutex is locked and dispatching is * enabled when we get here. We want to be able to run the @@ -250,33 +321,25 @@ bool _Thread_Initialize( * Mutex provides sufficient protection to let the user extensions * run safely. */ - extension_status = _User_extensions_Thread_create( the_thread ); - if ( extension_status ) - return true; + return _User_extensions_Thread_create( the_thread ); +} -#if defined(RTEMS_SMP) - while ( scheduler_index > 0 ) { - scheduler_node_for_index = (Scheduler_Node *) - ( (uintptr_t) scheduler_node_for_index - _Scheduler_Node_size ); - --scheduler_for_index; - --scheduler_index; - _Scheduler_Node_destroy( scheduler_for_index, scheduler_node_for_index ); - } -#else - if ( scheduler_index > 0 ) { - _Scheduler_Node_destroy( config->scheduler, scheduler_node ); - } -#endif +Status_Control _Thread_Initialize( + Thread_Information *information, + Thread_Control *the_thread, + const Thread_Configuration *config +) +{ + bool ok; - _Freechain_Push( - &information->Thread_queue_heads.Free, - the_thread->Wait.spare_heads - ); + ok = _Thread_Try_initialize( information, the_thread, config ); -#if defined(RTEMS_SMP) || CPU_ENABLE_ROBUST_THREAD_DISPATCH == TRUE -failed: -#endif + if ( !ok ) { + _Objects_Close( &information->Objects, &the_thread->Object ); + _Thread_Free( information, the_thread ); - ( *the_thread->Start.stack_free )( the_thread->Start.Initial_stack.area ); - return false; + return STATUS_UNSATISFIED; + } + + return STATUS_SUCCESSFUL; } diff --git a/cpukit/score/src/threadrestart.c b/cpukit/score/src/threadrestart.c index cf92e25a5d..e56eaa9fc8 100644 --- a/cpukit/score/src/threadrestart.c +++ b/cpukit/score/src/threadrestart.c @@ -155,52 +155,6 @@ static void _Thread_Make_zombie( Thread_Control *the_thread ) _Thread_Wake_up_joining_threads( the_thread ); } -static void _Thread_Free( Thread_Control *the_thread ) -{ - Thread_Information *information; - - _User_extensions_Thread_delete( the_thread ); - _User_extensions_Destroy_iterators( the_thread ); - _ISR_lock_Destroy( &the_thread->Keys.Lock ); - _Scheduler_Node_destroy( - _Thread_Scheduler_get_home( the_thread ), - _Thread_Scheduler_get_home_node( the_thread ) - ); - _ISR_lock_Destroy( &the_thread->Timer.Lock ); - - /* - * The thread might have been FP. So deal with that. - */ -#if ( CPU_HARDWARE_FP == TRUE ) || ( CPU_SOFTWARE_FP == TRUE ) -#if ( CPU_USE_DEFERRED_FP_SWITCH == TRUE ) - if ( _Thread_Is_allocated_fp( the_thread ) ) - _Thread_Deallocate_fp(); -#endif -#endif - - information = _Thread_Get_objects_information( the_thread ); - _Freechain_Push( - &information->Thread_queue_heads.Free, - the_thread->Wait.spare_heads - ); - - /* - * Free the rest of the memory associated with this task - * and set the associated pointers to NULL for safety. - */ - ( *the_thread->Start.stack_free )( the_thread->Start.Initial_stack.area ); - -#if defined(RTEMS_SMP) - _ISR_lock_Destroy( &the_thread->Scheduler.Lock ); - _ISR_lock_Destroy( &the_thread->Wait.Lock.Default ); - _SMP_lock_Stats_destroy( &the_thread->Potpourri_stats ); -#endif - - _Thread_queue_Destroy( &the_thread->Join_queue ); - _Context_Destroy( the_thread, &the_thread->Registers ); - _Objects_Free( &information->Objects, &the_thread->Object ); -} - static void _Thread_Wait_for_execution_stop( Thread_Control *the_thread ) { #if defined(RTEMS_SMP) @@ -227,10 +181,13 @@ void _Thread_Kill_zombies( void ) the_thread = (Thread_Control *) _Chain_Get_unprotected( &zombies->Chain ); while ( the_thread != NULL ) { + Thread_Information *information; + _ISR_lock_Release_and_ISR_enable( &zombies->Lock, &lock_context ); _Thread_Wait_for_execution_stop( the_thread ); - _Thread_Free( the_thread ); + information = _Thread_Get_objects_information( the_thread ); + _Thread_Free( information, the_thread ); _ISR_lock_ISR_disable_and_acquire( &zombies->Lock, &lock_context ); -- cgit v1.2.3