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

@@ -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

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.
  *

cpukit/posix/src/pthreadcreate.c

@@ -26,6 +26,7 @@
 #include <pthread.h>
 #include <errno.h>
 
+#include <rtems/posix/posixapi.h>
 #include <rtems/posix/priorityimpl.h>
 #if defined(RTEMS_POSIX_API)
 #include <rtems/posix/psignalimpl.h>
@@ -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 );
+  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;
     }

cpukit/rtems/src/taskconstruct.c

@@ -25,6 +25,7 @@
 #include <rtems/rtems/eventimpl.h>
 #include <rtems/rtems/modesimpl.h>
 #include <rtems/rtems/support.h>
+#include <rtems/rtems/statusimpl.h>
 #include <rtems/score/apimutex.h>
 #include <rtems/score/schedulerimpl.h>
 #include <rtems/score/stackimpl.h>
@@ -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;
   }

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 );

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

cpukit/score/src/threadinitialize.c

@@ -29,14 +29,81 @@
 #include <rtems/score/watchdogimpl.h>
 #include <rtems/config.h>
 
-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;
-
-#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
-
-  _Freechain_Push(
-    &information->Thread_queue_heads.Free,
-    the_thread->Wait.spare_heads
-  );
-
-#if defined(RTEMS_SMP) || CPU_ENABLE_ROBUST_THREAD_DISPATCH == TRUE
-failed:
-#endif
-
-  ( *the_thread->Start.stack_free )( the_thread->Start.Initial_stack.area );
-  return false;
+  return _User_extensions_Thread_create( the_thread );
+}
+
+Status_Control _Thread_Initialize(
+  Thread_Information         *information,
+  Thread_Control             *the_thread,
+  const Thread_Configuration *config
+)
+{
+  bool ok;
+
+  ok = _Thread_Try_initialize( information, the_thread, config );
+
+  if ( !ok ) {
+    _Objects_Close( &information->Objects, &the_thread->Object );
+    _Thread_Free( information, the_thread );
+
+    return STATUS_UNSATISFIED;
+  }
+
+  return STATUS_SUCCESSFUL;
 }

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 );