Skip to content

gstvideodecoder: Fix racy critical when pool negotiation occurs during flush

Alicia Boya García requested to merge ntrrgc/gst-plugins-base:basedecoder-pool-negotiation-race into master

I found a rather reproducible race in a WebKit LayoutTest when a player was intantiated and a VP8/9 video was loaded, then torn down after getting the video dimensions from the caps.

The crash occurs during the handling of the first frame by gstvpxdec. The following actions happen sequentially leading to a crash.

(MT=Main Thread, ST=Streaming Thread)

  1. MT: Sets pipeline state to NULL, which deactivates vpxdec's srcpad, which in turn sets its FLUSHING flag.

  2. ST: gst_vpx_dec_handle_frame() -- which is still running -- calls gst_video_decoder_allocate_output_frame(); this in turn calls gst_video_decoder_negotiate_unlocked() which fails because the srcpad is FLUSHING. As a direct consequence of the negotiation failure, a pool is NOT set.

    gst_video_decoder_negotiate_unlocked() still assumes there is a pool, crashing in a critical in gst_buffer_pool_acquire_buffer() a couple statements later.

This patch fixes the bug by returning != GST_FLOW_OK when the negotiation fails. If the srcpad is FLUSHING, GST_FLOW_FLUSHING is returned, otherwise GST_FLOW_ERROR is used.

Edited by Alicia Boya García

Merge request reports