[Bug 750397] New: CRITICAL: Race condition in GstBus

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=750397

            Bug ID: 750397
           Summary: CRITICAL: Race condition in GstBus
    Classification: Platform
           Product: GStreamer
           Version: 1.4.5
                OS: All
            Status: NEW
          Severity: critical
          Priority: Normal
         Component: gstreamer (core)
          Assignee: gstreamer-bugs at lists.freedesktop.org
          Reporter: dtverdunov at elvees.com
        QA Contact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---

gst_bus_post function has race condition. When one thread posts messages to a
bus and another one reads them from this bus, the following situation is
possible:

1. The first thread posts a message using gst_bus_post and GST_BUS_PASS. This
leads to gst_atomic_queue_push being called (line 346 gstbus.c). Then it calls
gst_poll_write_control (line 347 gstbus.c). gst_poll_write_control calls
raise_wakeup (gstpoll.c, line 180). raise_wakeup increments control_pending
variable and then calls WAKE_EVENT macro, as control_pending was 0 before
increment. But wait! Imagine that WAKE_EVENT is not called now, because the CPU
has just passed execution to the second thread, which calls
gst_bus_timed_pop_filtered.

2. This second thread calls gst_bus_timed_pop_filtered, which calls
gst_atomic_queue_pop (line 499 gstbus.c). We have 1 message in the queue
already. Right after this it also calls gst_poll_read_control (gstpoll.c, line
1541), which calls release_wakeup (gstpoll.c, line 196), which decrements
set->control_pending value, making it zero again. This means, that
RELEASE_EVENT macro is called. It reads a byte from the socket (note that
there's nothing in the socket yet).

3. Now the first thread continues its execution. It calls WAKE_EVENT, which
writes a byte to the socket.

At this moment we have empty bus queue, control_pending == 0, but the byte in
the socket! This means, every call to gst_bus_timed_pop_filtered from this
moment will lead to immediate wakeup of function gst_poll_wait, as the socket
has data to read. But we have no messages in the queue! Now the code like this
will execute without any delays, even if there're no messages in the queue:

do
{
  GstMessage* msg = gst_bus_timed_pop_filtered(bus, 100 * GST_MSECOND,
GST_MESSAGE_ANY);
  //Process message
}
while(1);


This mutex fixes the issue:

    case GST_BUS_PASS:
      /* pass the message to the async queue, refcount passed in the queue */
      GST_DEBUG_OBJECT (bus, "[msg %p] pushing on async queue", message);
      g_mutex_lock (&bus->priv->queue_lock); //PROPOSED FIX
      gst_atomic_queue_push (bus->priv->queue, message);
      gst_poll_write_control (bus->priv->poll);
      g_mutex_unlock (&bus->priv->queue_lock); //PROPOSED FIX
      GST_DEBUG_OBJECT (bus, "[msg %p] pushed on async queue", message);


Probably GST_BUS_ASYNC case has race condition, too.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.