After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755232 - dashdemux: demux->cancelled is not properly protected
dashdemux: demux->cancelled is not properly protected
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 755169
Reported: 2015-09-18 16:36 UTC by Florin Apostol
Modified: 2015-10-29 11:07 UTC
See Also:
GNOME target: ---
GNOME version: ---

unlock mutexes in reverse locking order (1.17 KB, patch)
2015-09-21 15:14 UTC, Vincent Penquerc'h
none Details | Review

Description Florin Apostol 2015-09-18 16:36:51 UTC
When a variable is shared between 2 threads, both read and write operations on the variable must be performed with the same lock taken.

I've done an analysis on how demux->cancelled variable is locked before access:

write access, variable set to FALSE
gst_adaptive_demux_start_tasks: GST_MANIFEST_LOCK

write access, variable set to TRUE
gst_adaptive_demux_stop_tasks: no lock taken!

read accesses:
gst_adaptive_demux_stream_wait_manifest_update: GST_MANIFEST_LOCK
gst_adaptive_demux_stream_download_uri: stream->fragment_download_lock
gst_adaptive_demux_stream_download_header_fragment: no lock taken!
gst_adaptive_demux_stream_download_loop: sometimes GST_OBJECT_LOCK (demux) + GST_MANIFEST_LOCK, sometimes GST_OBJECT_LOCK (demux), sometimes fragment_download_lock

Seems that different readers use different locks and the writer (gst_adaptive_demux_stop_tasks) does not use any lock!

What lock should be used for this variable? Or gst_adaptive_demux_stop_tasks function should grab all 3 locks when setting it?
Comment 1 Vincent Penquerc'h 2015-09-21 15:14:50 UTC
Created attachment 311767 [details] [review]
unlock mutexes in reverse locking order

Found while looking at how the cancelled variable is used.
Comment 2 Vincent Penquerc'h 2015-10-15 15:17:10 UTC
This is fixed as a side effect of Florian's patch from