Race condition in _Thread_Change_priority()

[Sebastian Huber]

We have:

void _Thread_Change_priority(
  Thread_Control   *the_thread,
  Priority_Control  new_priority,
  bool              prepend_it
)
{
  /*
   *  Do not bother recomputing all the priority related information if
   *  we are not REALLY changing priority.
   */
  if ( the_thread->current_priority != new_priority ) {
    ISR_Level level;

    _ISR_Disable( level );

    the_thread->current_priority = new_priority;

    if ( _States_Is_ready( the_thread->current_state ) ) {
      _Scheduler_Change_priority(
        the_thread,
        new_priority,
        prepend_it
      );
    } else {
      _Scheduler_Update_priority( the_thread, new_priority );
    }

    _ISR_Enable( level );

Here we changed the the_thread->current_priority and provided the thread is in a priority queue (RB tree), then its position in the tree is wrong now. In case it is the first or last element in the tree, then extract operations may corrupt the tree (e.g. if a timeout happens now).

    _Thread_queue_Requeue( the_thread->Wait.queue, the_thread );
  }
}

A potential solution is to set the_thread->current_priority and call _Thread_queue_Requeue() in one critical section. The scheduler is more resilient to temporarily wrong priority values, since it maintains its own priority depending state.

[Joel Sherrill]

Off the top of my head, I don't see a more expedient solution. It does negatively impact interrupt latency but this is still probably not the worst place.

Before thread queue priority was changed to rbtree, the worst was actually the insertion on priority on architectures where memory operations are more expensive than math operations. On architectures with poor bitscan and memory operations of lower overhead, the blocking path where a bit scan is needed to find a new heir was it.

[Sebastian Huber]

[6a941e3a9986d3cfb3e4ed0139e983b0455cc73b/rtems]