[Bug 742684] aggregator: Usage of GCond is racy.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=742684
  GStreamer | gst-plugins-bad | git

--- Comment #6 from Olivier Crête <olivier.crete at ocrete.ca> 2015-01-10 19:35:08 UTC ---
Actually, I though the lack of a loop was the problem, but there is one, it's
just well hidden by the fact that the aggregate function is called in a loop.

The problem is actually that a bunch of the member variables that are used in
multiple threads are sometimes modified with some lock held and sometimes not.
And every case where the SRC_STREAM_BROADCAST() macro is used is a case where
the variable that is checked before waiting on the GCond has been updated
without having the same mutex that is used by the GConf held and is therefore
racy.

Someone really needs to go over every member variable in GstAggregator and
document which mutex protects it (or why it doesn't need it!)

The only safe pattern to use a GCond is:

thread 1:
g_mutex_lock(mutex);
update variable ..; /* same mutex used to protect changes to the variable as
used when reading it and for the GCond ! */
g_cond_broadcast (cond);
g_mutex_unlock(mutex);

thread 2:
g_mutex_lock (mutex);
while (variable) {
  g_cond_wait (cond, mutex);
  /* it's allow to unlock and relock here ! */
}
g_mutex_unlock (mutex)


Also, there are probably too many added mutexes in GstAggregator: the
event_lock and "stream_lock" should the the Object lock of the pad, the setcaps
lock should be the srcpad stream lock (actually should always be called from
the streaming thread!).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.