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 737690 - rtsp-client: deadlock when setting session medias to NULL
rtsp-client: deadlock when setting session medias to NULL
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-30 23:35 UTC by Aleix Conchillo Flaqué
Modified: 2014-10-01 09:37 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
set session media to NULL without the client lock (1.45 KB, patch)
2014-09-30 23:43 UTC, Aleix Conchillo Flaqué 		none Details | Review
set session media to NULL without the client lock (1.45 KB, patch)
2014-10-01 05:31 UTC, Aleix Conchillo Flaqué 		committed Details | Review

Description Aleix Conchillo Flaqué 2014-09-30 23:35:28 UTC 
client_unwatch_session is called with the client lock and sets session medias to NULL with the lock acquired.

In my app I'm using RTP session "on-ssrc-stats" where I use 
gst_rtsp_client_session_filter.

So, if a media "on-ssrc-stats" happens while client_unwatch_session has also been called a deadlock occurs. That's because gst_rtsp_client_session_filter is waiting for the client lock and  gst_rtsp_media_unprepare is waiting for the pad that emitted "on-ssrc-stats".

I can't attach a backtrace since it point to some internal code which wouldn't make much sense here.
Comment 1 Aleix Conchillo Flaqué 2014-09-30 23:43:19 UTC 
Created attachment 287491 [details] [review]
set session media to NULL without the client lock

This patch fixes the problem by calling 

   gst_rtsp_session_filter (sess, filter_session_media, client);

inside cleanup_session where client lock is not acquired.

This also seems to solve other bugs and lots of assertion critical warnings (in a stress test with thousands of connection/reconnections).
Comment 2 Aleix Conchillo Flaqué 2014-09-30 23:47:17 UTC 
Talked to early. Just hit a different deadlock before click the send button.
Comment 3 Aleix Conchillo Flaqué 2014-10-01 00:29:10 UTC 
My new deadlock mentioned in comment 2 seems unrelated (just got much further than without the patch). Patch from comment 1 should still be valid.
Comment 4 Aleix Conchillo Flaqué 2014-10-01 05:22:51 UTC 
By the way, "on-ssrc-stats" is something I added with patch from bug 728918, but for the purpose of this bug it has exactly the same effect as "on-ssrc-active" (it is emitted from the same place).
Comment 5 Aleix Conchillo Flaqué 2014-10-01 05:31:01 UTC 
Created attachment 287492 [details] [review]
set session media to NULL without the client lock

Same patch with comment formatting fixup.
Comment 6 Luis de Bethencourt 2014-10-01 09:26:15 UTC 
Awesome work Aleix! :)
Comment 7 Luis de Bethencourt 2014-10-01 09:28:26 UTC 
Wim approves it. Merging it.
Comment 8 Luis de Bethencourt 2014-10-01 09:37:21 UTC 
Comment on attachment 287492 [details] [review]
set session media to NULL without the client lock

Merged