Skip to content

v4l2: Record buffer states in pool to fix dequeue race

The gst_v4l2_buffer_pool_dqbuf function contains this ominous comment:

/* get our GstBuffer with that index from the pool, if the buffer was
 * outstanding we have a serious problem.
 */
outbuf = pool->buffers[group->buffer.index];

Unfortunately it is common for buffers in output buffer pools to be both queued and outstanding at the same time. This can happen if the upstream element keeps a reference to the buffer, or in an encoder element itself when it keeps a reference to the input buffer for each frame.

Since the current code doesn't handle this case properly we can end up with crashes in other elements such as:

(gst-launch-1.0:32559): CRITICAL **: 17:33:35.740: gst_video_frame_map_id: assertion 'GST_IS_BUFFER (buffer)' failed

and:

(gst-launch-1.0:231): GStreamer-CRITICAL **: 00:16:20.882: write map requested on non-writable buffer

Both these crashes are caused by a race condition related to releasing the same buffer twice from two different threads. If a buffer is queued and outstanding this situation is possible:

Thread 1

  • Calls gst_buffer_unref decrementing the reference count to zero.
  • The core GstBufferPool object marks the buffer non-outstanding.
  • Calls the V4L2 release buffer function.
  • If the buffer is not queued:
    • Release it back to the free pool (containing non-queued buffers).

Thread 2

  • Dequeues the queued output buffer.
    • Marks the buffer as not queued.
  • If the buffer is not outstanding:
    • Calls the V4L2 release buffer function.
    • Release it back to the free pool (containing non-queued buffers).

If both of these threads run at exactly the same time there is a small window where the buffer is marked both not outstanding and not queued but before it has been released. In this case the buffer will be freed twice causing the above crashes.

Unfortunately the variable recording whether a buffer is outstanding is part of the core GstBuffer object and is managed by GstBufferPool so it's not as straightforward as adding a mutex. Instead we can fix this by additionally recording the buffer state in GstV4l2BufferPool, and handle "internal" and "external" buffer release separately so we can detect when a buffer becomes not outstanding.

In the new solution:

  • The "external" buffer pool release and the "dqbuf" functions atomically update the buffer state and determine if a buffer is still queued or outstanding.
  • Subsequent code and a new gst_v4l2_buffer_pool_complete_release_buffer function can proceed to release (or not) a buffer knowing that it's not racing with another thread.

Merge request reports