[x264-devel] [PATCH] Fix race condition in win32 threading init

[Sérgio Basto]

JFTR 

Delivery-Date: Thu, 20 Apr 2017 09:00:10 +0100


On Qui, 2017-02-23 at 12:54 +0300, Andrey Turkin wrote:
> x264_threading_init must not return until threading is fully
> initialized. The code is mostly borrowed from FFmpeg codebase and
> adapted to handle init failure case.
> 
> ---
>  common/osdep.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
> ------------
>  1 file changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/common/osdep.c b/common/osdep.c
> index 9bc926a1..f07d446a 100644
> --- a/common/osdep.c
> +++ b/common/osdep.c
> @@ -40,6 +40,12 @@
>  #endif
>  #include <time.h>
>  
> +#if _WIN32_WINNT < 0x0600 && defined(__MINGW32__)
> +#undef MemoryBarrier
> +#define MemoryBarrier __sync_synchronize
> +#endif
> +
> +
>  #if PTW32_STATIC_LIB
>  /* this is a global in pthread-win32 to indicate if it has been
> initialized or not */
>  extern int ptw32_processInitialized;
> @@ -74,23 +80,55 @@ static void x264_threading_destroy( void )
>  
>  int x264_threading_init( void )
>  {
> -    /* if already init, then do nothing */
> -    if( InterlockedCompareExchange( &x264_threading_is_init, 1, 0 )
> )
> -        return 0;
> +    switch( InterlockedCompareExchange( &x264_threading_is_init, 1,
> 0 ) )
> +    {
> +    case 0:
> +        /* this thread has to initialize threading */
>  #if PTW32_STATIC_LIB
> -    /* if static pthread-win32 is already initialized, then do
> nothing */
> -    if( ptw32_processInitialized )
> -        return 0;
> -    if( !pthread_win32_process_attach_np() )
> -        return -1;
> +        /* if static pthread-win32 is already initialized, then do
> nothing */
> +        if( !ptw32_processInitialized )
> +        {
> +            if( !pthread_win32_process_attach_np() )
> +            {
> +                InterlockedExchange( &x264_threading_is_init, 3 );
> +                return -1;
> +            }
> +            /* register cleanup to run at process termination */
> +            atexit( x264_threading_destroy );
> +        }
>  #else
> -    if( x264_win32_threading_init() )
> -        return -1;
> +        if( x264_win32_threading_init() )
> +        {
> +            InterlockedExchange( &x264_threading_is_init, 3 );
> +            return -1;
> +        }
> +        /* register cleanup to run at process termination */
> +        atexit( x264_threading_destroy );
>  #endif
> -    /* register cleanup to run at process termination */
> -    atexit( x264_threading_destroy );
> +        InterlockedExchange( &x264_threading_is_init, 2 );
> +        return 0;
>  
> -    return 0;
> +    case 1:
> +        /* other thread is running init */
> +        for( ;; )
> +        {
> +            MemoryBarrier();
> +            if( x264_threading_is_init == 2 )
> +                return  0;
> +            if( x264_threading_is_init == 3 )
> +                return  -1;
> +            Sleep( 0 );
> +        }
> +
> +    case 2:
> +        /* already initialized */
> +        return 0;
> +
> +    case 3:
> +        /* initialize failed */
> +    default:
> +        return -1;
> +    }
>  }
>  #endif
>  
-- 
Sérgio M. B.