[PATCH 2/2] score: PR2140: Fix _Thread_queue_Process_timeout()

[Gedare Bloom]

This seems OK. I have not looked deeply enough but it bugs me that
maybe there is a race condition on the_thread->Wait.return_code also?

-Gedare

On Fri, Aug 23, 2013 at 6:58 AM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> The _Thread_queue_Process_timeout() operation had several race
> conditions in the event of nested interrupts.  Protect the critical
> sections via disabled interrupts.
> ---
>  cpukit/score/src/threadqprocesstimeout.c |   37 +++++++++++++++++++++++------
>  1 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/cpukit/score/src/threadqprocesstimeout.c b/cpukit/score/src/threadqprocesstimeout.c
> index 6aaf445..11df85b 100644
> --- a/cpukit/score/src/threadqprocesstimeout.c
> +++ b/cpukit/score/src/threadqprocesstimeout.c
> @@ -25,7 +25,8 @@ void _Thread_queue_Process_timeout(
>    Thread_Control *the_thread
>  )
>  {
> -  Thread_queue_Control *the_thread_queue = the_thread->Wait.queue;
> +  Thread_queue_Control *the_thread_queue;
> +  ISR_Level             level;
>
>    /*
>     *  If the_thread_queue is not synchronized, then it is either
> @@ -39,15 +40,35 @@ void _Thread_queue_Process_timeout(
>     *  a timeout is not allowed to occur.
>     */
>
> -  if ( the_thread_queue->sync_state != THREAD_BLOCKING_OPERATION_SYNCHRONIZED &&
> -       _Thread_Is_executing( the_thread ) ) {
> -    if ( the_thread_queue->sync_state != THREAD_BLOCKING_OPERATION_SATISFIED ) {
> -      the_thread->Wait.return_code = the_thread->Wait.queue->timeout_status;
> -      the_thread_queue->sync_state = THREAD_BLOCKING_OPERATION_TIMEOUT;
> +  _ISR_Disable( level );
> +  the_thread_queue = the_thread->Wait.queue;
> +  if ( the_thread_queue != NULL ) {
> +    if ( the_thread_queue->sync_state != THREAD_BLOCKING_OPERATION_SYNCHRONIZED &&
> +         _Thread_Is_executing( the_thread ) ) {
> +      if ( the_thread_queue->sync_state != THREAD_BLOCKING_OPERATION_SATISFIED ) {
> +        the_thread->Wait.return_code = the_thread_queue->timeout_status;
> +        the_thread_queue->sync_state = THREAD_BLOCKING_OPERATION_TIMEOUT;
> +      }
> +      _ISR_Enable( level );
> +    } else {
> +      bool we_did_it;
> +
> +      _ISR_Enable( level );
> +
> +      /*
> +       * We can use the_thread_queue pointer here even if
> +       * the_thread->Wait.queue is already set to NULL since the extract
> +       * operation will only use the thread queue discipline to select the
> +       * right extract operation.  The timeout status is set during thread
> +       * queue initialization.
> +       */
> +      we_did_it = _Thread_queue_Extract( the_thread_queue, the_thread );
> +      if ( we_did_it ) {
> +        the_thread->Wait.return_code = the_thread_queue->timeout_status;
> +      }
>      }
>    } else {
> -    the_thread->Wait.return_code = the_thread->Wait.queue->timeout_status;
> -    _Thread_queue_Extract( the_thread->Wait.queue, the_thread );
> +    _ISR_Enable( level );
>    }
>  }
>
> --
> 1.7.7
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel