GNOME Bugzilla – Bug 108263
Possible race condition in gstqueue.c
Last modified: 2004-12-22 21:47:04 UTC
In gstqueue.c, there appears to be the potential for a (highly unlikely, but not impossible) race condition in the gst_queue_chain() method when checking the 'flush' variable on a full queue: if (queue->interrupt) { GST_DEBUG_ELEMENT (GST_CAT_DATAFLOW, queue, "interrupted!!"); g_mutex_unlock (queue->qlock); if (gst_scheduler_interrupt (gst_pad_get_scheduler (queue->sinkpad), GST_ELEMENT (queue))) return; /* if we got here bacause we were unlocked after a flush, we don't need * to add the buffer to the queue again */ if (queue->flush) { GST_DEBUG_ELEMENT (GST_CAT_DATAFLOW, queue, "not adding pending buffer after flush"); return; } Note that the 'queue->flush' data member is tested after the mutex is unlocked. This should probably be copied to an automatic variable before the call to g_mutex_unlock(); see the attached patch.
Created attachment 14981 [details] [review] Patch for unlikely race condition in gstqueue.c
I believe the current behaviour is correct. Imagine the following scenario: - The queue is full. - A new buffer arrives. The chain function returns via gst_scheduler_interrupt to let the scheduler go on. - A flush event arrives on the other side of the queue, the queue is emptied. - The scheduler reschedules the queue, the chain function goes on executing. The queue has been flushed, but we're still holding a buffer. In this case the buffer should be discarded and not put into the queue, which is exactly what is done. As a sidenote: We're missing a lot of gst_buffer_unref's when returning from erroneous behaviour.
Makes sense; I agree that this is not, in fact a bug.