[Bug 750397] CRITICAL: Race condition in GstBus

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #330201|needs-work                  |none
             status|                            |
 Attachment #330201|0                           |1
        is obsolete|                            |

--- Comment #22 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Created attachment 330253
  --> https://bugzilla.gnome.org/attachment.cgi?id=330253&action=edit
poll: Take a mutex when the control state changes before signalling

The previous code was racy as too much protection was expected from the atomic
integer. What could have happened is:

Thread 1:
  - release()
  - --control_pending == 0

Thread 2:
  - raise()
  - control_pending++ == 0
  - WAKE_EVENT()

Thread 1:
  - RELEASE_EVENT()

The final state would be that control_pending > 0 but no event is pending. As
such, only when release() would be called another time everything could get
into a consistent state again. However e.g. if the GstPoll is used by a bus
GSource, the source would never wake up again and would have no opportunity to
call release() again.

The same thing can happen in the other way around:

Thread 1:
  - raise()
  - control_pending++ == 0

Thread 2:
  - release()
  - --control_pending == 0
  - RELEASE_EVENT()

Thread 1:
  - WAKE_EVENT()

Here the final state would be that control_pending == 0 but an event is
pending. Future calls to raise() would add *yet another* event (except on
Windows, where the event can only be signalled or not), but release() would
only release one of them. This results in there always being an event pending.
With GstBus this causes an empty bus to cause gst_bus_timed_pop_filtered() to
wake up immediately, see no message is there, sleep again, wake up again
immediately, repeat forever.

Note that this is also forbidden by the GstPoll API: read_control() must not
be called before write_control() has returned. Follow-up patches to GstBus and
GstBufferPool will fix this.

As a solution we add a mutex for the case when control_pending == 0 before
WAKE_EVENT() and RELEASE_EVENT(), and then have a second, lock protected
control_pending counter that reflects the real situation while everything
while everything is locked around the socket access.

While this mutex makes gst_bus_post() not completely lockless anymore, it is
only taken whenever we change from an empty to a non-empty bus and back. That
is, not very often ideally.

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