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 761399 - rtspmedia: Missing lock in default_unsuspend, preroll_failed path
rtspmedia: Missing lock in default_unsuspend, preroll_failed path
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Windows
: Normal normal
: 1.6.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
1.6.4
Depends on:
Blocks:
 
 
Reported: 2016-02-01 12:10 UTC by stevenhoving
Modified: 2016-04-14 17:42 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
rtsp-media: fix state_lock not locked again when preroll fails. (676 bytes, patch)
2016-02-01 12:15 UTC, stevenhoving 		none Details | Review
rtsp-media: fix state_lock not locked again when preroll fails v2. (1.23 KB, patch)
2016-02-02 08:04 UTC, stevenhoving 		committed Details | Review

Description stevenhoving 2016-02-01 12:10:43 UTC 
The function 'default_unsuspend' in rtsp-media.c unlocks and locks the state_lock in the 'GST_RTSP_SUSPEND_MODE_RESET' case handler. But when 'wait_preroll' fails, it will jump to the 'preroll_failed' label. Where the state_lock is not locked again.
Comment 1 stevenhoving 2016-02-01 12:15:56 UTC 
Created attachment 320178 [details] [review]
rtsp-media: fix state_lock not locked again when preroll fails.
Comment 2 Tim-Philipp Müller 2016-02-01 18:58:37 UTC 
Comment on attachment 320178 [details] [review]
rtsp-media: fix state_lock not locked again when preroll fails.

Thanks for the patch, looks good.

Could I persuade you to rewrite it as follows: add a gboolean preroll_ok so that the flow is:

  unlock()
  preroll_ok = wait_preroll()
  lock()

  if (!preroll_ok)
    goto fail;

? (I know we usually do unlocks and such also in the error code paths, but here it's easy to keep the locking pair in one place so it seems preferable to me to do that instead)
Comment 3 stevenhoving 2016-02-02 08:04:46 UTC 
Created attachment 320251 [details] [review]
rtsp-media: fix state_lock not locked again when preroll fails v2.
Comment 4 Tim-Philipp Müller 2016-02-02 10:41:46 UTC 
Thanks, pushed to master

commit aea624b6f8b93d506eb7b1f8a8b9086ca552856f
Author: Steven Hoving <sh@bigbrother.nl>
Date:   Tue Feb 2 09:01:51 2016 +0100

    rtsp-media: fix state_lock not locked again when preroll fails
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761399

and picked into 1.6 branch.