[PATCH] score: Fix _Thread_Cancel()

[Gedare Bloom]

On Thu, May 20, 2021 at 9:18 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> The _Thread_Cancel() (in contrast to _Thread_Restart() which used a
> similar code block) may have produced ready threads with an active timer
> in case the thread to cancel had its thread life protection enabled. The
> problem was this code block:
>
>     Priority_Control priority;
>
>     _Thread_Add_life_change_request( the_thread );
>
>     if ( _Thread_Is_life_change_allowed( previous ) ) {
>       _Thread_State_release( the_thread, &lock_context );
>
>       _Thread_queue_Extract_with_proxy( the_thread );
>       _Thread_Timer_remove( the_thread );
>     } else {
>       _Thread_Clear_state_locked( the_thread, STATES_SUSPENDED );
>       _Thread_State_release( the_thread, &lock_context );
>     }
>
>     priority = _Thread_Get_priority( executing );
>     _Thread_Raise_real_priority( the_thread, priority );
>     _Thread_Remove_life_change_request( the_thread );
>
> The life change request should only be added/removed if a life change is
> allowed (see _Thread_Restart()).  Add
> _Thread_Consider_life_change_request() and use it in _Thread_Cancel()
> and _Thread_Restart().
>
> Close #4435.
> ---
>  cpukit/score/src/threadrestart.c | 50 ++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/cpukit/score/src/threadrestart.c b/cpukit/score/src/threadrestart.c
> index 933faee61b..58fcd89159 100644
> --- a/cpukit/score/src/threadrestart.c
> +++ b/cpukit/score/src/threadrestart.c
> @@ -403,6 +403,25 @@ static void _Thread_Set_exit_value(
>    the_thread->Life.exit_value = exit_value;
>  }
>
> +static void _Thread_Consider_life_change_request(

Minor, I'd prefer "_Thread_Try_life_change_request("

> +  Thread_Control    *the_thread,
> +  Thread_Life_state  previous,
> +  ISR_lock_Context  *lock_context
> +)
> +{
> +  if ( _Thread_Is_life_change_allowed( previous ) ) {
> +    _Thread_Add_life_change_request( the_thread );
> +    _Thread_State_release( the_thread, lock_context );
> +
> +    _Thread_queue_Extract_with_proxy( the_thread );
> +    _Thread_Timer_remove( the_thread );
> +    _Thread_Remove_life_change_request( the_thread );
> +  } else {
> +    _Thread_Clear_state_locked( the_thread, STATES_SUSPENDED );
> +    _Thread_State_release( the_thread, lock_context );
> +  }
> +}
> +
>  void _Thread_Cancel(
>    Thread_Control *the_thread,
>    Thread_Control *executing,
> @@ -433,21 +452,13 @@ void _Thread_Cancel(
>    } else {
>      Priority_Control priority;
>
> -    _Thread_Add_life_change_request( the_thread );
> -
> -    if ( _Thread_Is_life_change_allowed( previous ) ) {
> -      _Thread_State_release( the_thread, &lock_context );
> -
> -      _Thread_queue_Extract_with_proxy( the_thread );
> -      _Thread_Timer_remove( the_thread );
> -    } else {
> -      _Thread_Clear_state_locked( the_thread, STATES_SUSPENDED );
> -      _Thread_State_release( the_thread, &lock_context );
> -    }
> -
> +    _Thread_Consider_life_change_request(
> +      the_thread,
> +      previous,
> +      &lock_context
> +    );
>      priority = _Thread_Get_priority( executing );
>      _Thread_Raise_real_priority( the_thread, priority );
> -    _Thread_Remove_life_change_request( the_thread );
>    }
>
>    _Thread_Dispatch_enable( cpu_self );
> @@ -559,18 +570,7 @@ Status_Control _Thread_Restart(
>      THREAD_LIFE_RESTARTING,
>      ignored_life_states
>    );
> -
> -  if ( _Thread_Is_life_change_allowed( previous ) ) {
> -    _Thread_Add_life_change_request( the_thread );
> -    _Thread_State_release( the_thread, lock_context );
> -
> -    _Thread_queue_Extract_with_proxy( the_thread );
> -    _Thread_Timer_remove( the_thread );
> -    _Thread_Remove_life_change_request( the_thread );
> -  } else {
> -    _Thread_Clear_state_locked( the_thread, STATES_SUSPENDED );
> -    _Thread_State_release( the_thread, lock_context );
> -  }
> +  _Thread_Consider_life_change_request( the_thread, previous, lock_context );
>
>    _Thread_queue_Context_initialize( &queue_context );
>    _Thread_queue_Context_clear_priority_updates( &queue_context );
> --
> 2.26.2
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel