[x264-devel] [PATCH] This change moves the read of pool->exit to within mutex scope. Writes to the variable are already within mutex scope.

[BugMaster]

On Tue, 11 Dec 2018 05:47:05 -0800, Daniel Deptford wrote:
> ---
>  common/threadpool.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

> diff --git a/common/threadpool.c b/common/threadpool.c
> index 7f98f778..80b0f99b 100644
> --- a/common/threadpool.c
> +++ b/common/threadpool.c
> @@ -52,7 +52,8 @@ static void *threadpool_thread_internal( x264_threadpool_t *pool )
>      if( pool->init_func )
>          pool->init_func( pool->init_arg );
>  
-    while( !pool->>exit )
> +    int exit = 0;
> +    while( !exit )
>      {
>          x264_threadpool_job_t *job = NULL;
>          x264_pthread_mutex_lock( &pool->run.mutex );
> @@ -63,6 +64,7 @@ static void *threadpool_thread_internal( x264_threadpool_t *pool )
>              job = (void*)x264_frame_shift( pool->run.list );
>              pool->run.i_size--;
>          }
> +        exit = pool->exit;
>          x264_pthread_mutex_unlock( &pool->run.mutex );
>          if( !job )
>              continue;

Hi.

IMHO x264_threadpool_t->exit should simply be marked as volatile and
used as atomic variable like x264_lookahead_t->b_exit_thread.
Because mutex is there to protect x264_sync_frame_list_t itself and
not variables outside it. As for writing to this variable mutex is
locked not to protect it but to make this change atomic with
x264_pthread_cond_broadcast signaling.