#2310 closed defect (fixed)

Race condition in _Thread_Change_priority()

Reported by: Sebastian Huber Owned by:
Priority: normal Milestone: 4.11
Component: score Version: 4.11
Severity: critical Keywords:
Cc: Blocked By:
Blocking:

Description (last modified by 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.

Change History (3)

comment:1 Changed on Mar 17, 2015 at 2:29:00 PM by Sebastian Huber

Description: modified (diff)

comment:2 Changed on Mar 17, 2015 at 2:41:43 PM by 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.

comment:3 Changed on Mar 20, 2015 at 8:24:39 AM by Sebastian Huber

Milestone: 4.11.14.11
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.