Skip to content

[Bug fix] Prevent mb_info_free to be called before all threads are done using mb_info

Elias Carotti requested to merge Ytsejam76/x264:master into master

Please find attached a bug fix for a race condition on the mb_info_free callback.

Currently the code looks like the following (encoder/encoder.c):

           x264_threadslice_cond_broadcast( h, 2 );
            /* Do the first row of hpel, now that the previous slice is done */
            if( h->i_thread_idx > 0 )
            {
                x264_threadslice_cond_wait( h->thread[h->i_thread_idx-1], 2 );
                fdec_filter_row( h, h->i_threadslice_start + (1 << SLICE_MBAFF), 2 );
            }
      [....]
      if( h->fdec->mb_info_free && (!h->param.b_sliced_threads || h->i_thread_idx == (h->param.i_threads-1)) )
        {
            h->fdec->mb_info_free( h->fdec->mb_info );
            h->fdec->mb_info = NULL;
            h->fdec->mb_info_free = NULL;
        }
    }

Consider a case with at least three threads:

  1. Thread 0 is lagging behind the other threads and still in the x264_macroblock_analyse(...) method.
  2. Thread 1 is almost done and signals thread 2, then waits on thread 0 to signal in turn.
  3. Thread 2 receives the green from thread 1 and goes on to call mb_info_free(...).
  4. Thread 0 gets a SEGV if it checks for mb_info in the x264_macroblock_analyse(...).

A fix is for each thread to first wait (except for thread 0) then signal.

Edited by Elias Carotti

Merge request reports