score: Fix and simplify thread wait locks

There was a subtile race condition in _Thread_queue_Do_extract_locked().
It must first update the thread wait flags and then restore the default
thread wait state.  In the previous implementation this could lead under
rare timing conditions to an ineffective _Thread_Wait_tranquilize()
resulting to a corrupt system state.

Update #2556.

4 files changed, 46 insertions, 75 deletions

diff --git a/cpukit/score/src/threadchangepriority.c b/cpukit/score/src/threadchangepriority.c
index c569530d74..97e7950693 100644
--- a/cpukit/score/src/threadchangepriority.c
+++ b/cpukit/score/src/threadchangepriority.c
@@ -46,11 +46,12 @@ static Thread_Control *_Thread_Apply_priority_locked(
    *  we are not REALLY changing priority.
    */
   if ( ( *filter )( the_thread, &new_priority, arg ) ) {
-    _Thread_queue_Context_priority_change(
-      queue_context,
+    _Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
+
+    ( *the_thread->Wait.operations->priority_change )(
+      the_thread->Wait.queue,
       the_thread,
-      new_priority,
-      prepend_it
+      new_priority
     );
   } else {
     the_thread = NULL;
diff --git a/cpukit/score/src/threadinitialize.c b/cpukit/score/src/threadinitialize.c
index 209be9ebb1..b5fef590fc 100644
--- a/cpukit/score/src/threadinitialize.c
+++ b/cpukit/score/src/threadinitialize.c
@@ -186,7 +186,7 @@ bool _Thread_Initialize(
     &the_thread->Wait.Lock.Default,
     "Thread Wait Default Lock"
   );
-  _Chain_Initialize_empty( &the_thread->Wait.Lock.Pending_requests );
+  _Thread_queue_Gate_open( &the_thread->Wait.Lock.Tranquilizer );
   _SMP_lock_Stats_initialize( &the_thread->Potpourri_stats, "Thread Potpourri" );
 #endif
 
diff --git a/cpukit/score/src/threadqenqueue.c b/cpukit/score/src/threadqenqueue.c
index a9b2c35af5..9ac57d05b4 100644
--- a/cpukit/score/src/threadqenqueue.c
+++ b/cpukit/score/src/threadqenqueue.c
@@ -176,7 +176,7 @@ static void _Thread_queue_Path_release( Thread_queue_Path *path )
 
     link = RTEMS_CONTAINER_OF( node, Thread_queue_Link, Path_node );
 
-    if ( link->Queue_context.Wait.queue_lock != NULL ) {
+    if ( link->Queue_context.Wait.queue != NULL ) {
       _Thread_queue_Link_remove( link );
     }
 
@@ -213,8 +213,6 @@ static bool _Thread_queue_Path_acquire(
    */
 
   _Chain_Initialize_empty( &path->Links );
-  _Chain_Initialize_node( &path->Start.Path_node );
-  _Thread_queue_Context_initialize( &path->Start.Queue_context );
 
   owner = queue->owner;
 
@@ -226,6 +224,8 @@ static bool _Thread_queue_Path_acquire(
     return false;
   }
 
+  _Chain_Initialize_node( &path->Start.Path_node );
+  _Thread_queue_Context_initialize( &path->Start.Queue_context );
   link = &path->Start;
 
   do {
@@ -239,34 +239,36 @@ static bool _Thread_queue_Path_acquire(
 
     target = owner->Wait.queue;
     link->Queue_context.Wait.queue = target;
-    link->Queue_context.Wait.operations = owner->Wait.operations;
 
     if ( target != NULL ) {
       if ( _Thread_queue_Link_add( link, queue, target ) ) {
-        link->Queue_context.Wait.queue_lock = &target->Lock;
-        _Chain_Append_unprotected(
+        _Thread_queue_Gate_add(
           &owner->Wait.Lock.Pending_requests,
-          &link->Queue_context.Wait.Gate.Node
+          &link->Queue_context.Wait.Gate
         );
         _Thread_Wait_release_default_critical(
           owner,
           &link->Queue_context.Lock_context
         );
-        _Thread_Wait_acquire_queue_critical(
-          &target->Lock,
-          &link->Queue_context
-        );
+        _Thread_Wait_acquire_queue_critical( target, &link->Queue_context );
 
         if ( link->Queue_context.Wait.queue == NULL ) {
+          _Thread_queue_Link_remove( link );
+          _Thread_Wait_release_queue_critical( target, &link->Queue_context );
+          _Thread_Wait_acquire_default_critical(
+            owner,
+            &link->Queue_context.Lock_context
+          );
+          _Thread_Wait_remove_request_locked( owner, &link->Queue_context );
+          _Assert( owner->Wait.queue == NULL );
           return true;
         }
       } else {
-        link->Queue_context.Wait.queue_lock = NULL;
+        link->Queue_context.Wait.queue = NULL;
         _Thread_queue_Path_release( path );
         return false;
       }
     } else {
-      link->Queue_context.Wait.queue_lock = NULL;
       return true;
     }
 
@@ -330,6 +332,7 @@ void _Thread_queue_Enqueue_critical(
   if ( !_Thread_queue_Path_acquire( the_thread, queue, &path ) ) {
     _Thread_Wait_restore_default( the_thread );
     _Thread_queue_Queue_release( queue, &queue_context->Lock_context );
+    _Thread_Wait_tranquilize( the_thread );
     ( *queue_context->deadlock_callout )( the_thread );
     return;
   }
@@ -504,14 +507,15 @@ void _Thread_queue_Extract_critical(
 
 void _Thread_queue_Extract( Thread_Control *the_thread )
 {
-  Thread_queue_Context queue_context;
+  Thread_queue_Context  queue_context;
+  Thread_queue_Queue   *queue;
 
   _Thread_queue_Context_initialize( &queue_context );
   _Thread_Wait_acquire( the_thread, &queue_context );
 
-  if (
-    _Thread_queue_Context_get_queue( &queue_context, the_thread ) != NULL
-  ) {
+  queue = the_thread->Wait.queue;
+
+  if ( queue != NULL ) {
     bool unblock;
 
     _Thread_Wait_remove_request( the_thread, &queue_context );
@@ -520,14 +524,14 @@ void _Thread_queue_Extract( Thread_Control *the_thread )
       _Thread_queue_MP_callout_do_nothing
     );
     unblock = _Thread_queue_Extract_locked(
-      _Thread_queue_Context_get_queue( &queue_context, the_thread ),
-      _Thread_queue_Context_get_operations( &queue_context, the_thread ),
+      queue,
+      the_thread->Wait.operations,
       the_thread,
       &queue_context.Lock_context
     );
     _Thread_queue_Unblock_critical(
       unblock,
-      _Thread_queue_Context_get_queue( &queue_context, the_thread ),
+      queue,
       the_thread,
       &queue_context.Lock_context
     );
diff --git a/cpukit/score/src/threadqops.c b/cpukit/score/src/threadqops.c
index 8059446578..d6e42fda8f 100644
--- a/cpukit/score/src/threadqops.c
+++ b/cpukit/score/src/threadqops.c
@@ -22,50 +22,25 @@
 #include <rtems/score/rbtreeimpl.h>
 #include <rtems/score/schedulerimpl.h>
 
-static void _Thread_queue_Default_priority_change(
+static void _Thread_queue_Do_nothing_priority_change(
+  Thread_queue_Queue *queue,
   Thread_Control     *the_thread,
-  Priority_Control    new_priority,
-  bool                prepend_it,
-  Thread_queue_Queue *queue
+  Priority_Control    new_priority
 )
 {
   (void) queue;
-
-  _Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
+  (void) the_thread;
+  (void) new_priority;
 }
 
 static void _Thread_queue_Do_nothing_extract(
   Thread_queue_Queue *queue,
-  Thread_Control    *the_thread
-)
-{
-  /* Do nothing */
-}
-
-#if defined(RTEMS_SMP)
-static void _Thread_queue_Stale_queue_priority_change(
-  Thread_Control     *the_thread,
-  Priority_Control    new_priority,
-  bool                prepend_it,
-  Thread_queue_Queue *queue
+  Thread_Control     *the_thread
 )
 {
-  ISR_lock_Context lock_context;
-
   (void) queue;
-
-  /*
-   * This operation is used to change the priority in case we have a thread
-   * queue context with a stale thread queue.  We own the thread queue lock of
-   * the former thread queue.  In addition, we need the thread wait default
-   * lock, see _Thread_Wait_restore_default().
-   */
-
-  _Thread_Wait_acquire_default_critical( the_thread, &lock_context );
-  _Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
-  _Thread_Wait_release_default_critical( the_thread, &lock_context );
+  (void) the_thread;
 }
-#endif
 
 static Thread_queue_Heads *_Thread_queue_Queue_enqueue(
   Thread_queue_Queue *queue,
@@ -216,10 +191,9 @@ static bool _Thread_queue_Priority_less(
 }
 
 static void _Thread_queue_Priority_priority_change(
+  Thread_queue_Queue *queue,
   Thread_Control     *the_thread,
-  Priority_Control    new_priority,
-  bool                prepend_it,
-  Thread_queue_Queue *queue
+  Priority_Control    new_priority
 )
 {
   Thread_queue_Heads          *heads = queue->heads;
@@ -227,8 +201,6 @@ static void _Thread_queue_Priority_priority_change(
 
   _Assert( heads != NULL );
 
-  _Scheduler_Thread_set_priority( the_thread, new_priority, prepend_it );
-
   priority_queue = _Thread_queue_Priority_queue( heads, the_thread );
 
   _RBTree_Extract(
@@ -389,11 +361,12 @@ static void _Thread_queue_Priority_inherit_enqueue(
     owner->priority_restore_hint = true;
     _Atomic_Fence( ATOMIC_ORDER_ACQ_REL );
 
-    _Thread_queue_Context_priority_change(
-      &path->Start.Queue_context,
+    _Scheduler_Thread_set_priority( owner, priority, false );
+
+    ( *owner->Wait.operations->priority_change )(
+      owner->Wait.queue,
       owner,
-      priority,
-      false
+      priority
     );
   } else {
     path->update_priority = NULL;
@@ -424,7 +397,7 @@ void _Thread_queue_Boost_priority(
 #endif
 
 const Thread_queue_Operations _Thread_queue_Operations_default = {
-  .priority_change = _Thread_queue_Default_priority_change,
+  .priority_change = _Thread_queue_Do_nothing_priority_change,
   .extract = _Thread_queue_Do_nothing_extract
   /*
    * The default operations are only used in _Thread_Change_priority() and
@@ -434,7 +407,7 @@ const Thread_queue_Operations _Thread_queue_Operations_default = {
 };
 
 const Thread_queue_Operations _Thread_queue_Operations_FIFO = {
-  .priority_change = _Thread_queue_Default_priority_change,
+  .priority_change = _Thread_queue_Do_nothing_priority_change,
   .enqueue = _Thread_queue_FIFO_enqueue,
   .extract = _Thread_queue_FIFO_extract,
   .first = _Thread_queue_FIFO_first
@@ -453,10 +426,3 @@ const Thread_queue_Operations _Thread_queue_Operations_priority_inherit = {
   .extract = _Thread_queue_Priority_extract,
   .first = _Thread_queue_Priority_first
 };
-
-#if defined(RTEMS_SMP)
-const Thread_queue_Operations _Thread_queue_Operations_stale_queue = {
-  .priority_change = _Thread_queue_Stale_queue_priority_change,
-  .extract = _Thread_queue_Do_nothing_extract
-};
-#endif