GNOME Bugzilla – Bug 750544
RTSP server: crashes when accessing freed session in keep alive callback on shutdown
Last modified: 2016-10-25 13:04:37 UTC
Created attachment 304753 [details] Sample RTSP server to reproduce the bug It is possible to cause a segfault in RTSP server (probably this is because of races, as the probability of segfault is not 100%). Please, look at the attached sample first. You can compile it to reproduce this bug easily: 1. Launch test RTSP server. 2. Open 6 VLC players and connect five of them to RTSP server. 3. Try to connect to the server in the last 6th VLC, you will receive an error, as the maximum number of sessions is limited to 5. 4. Now stop playing in one of connected VLC players. RTSP server will almost certainly crash. This bug can be also reproduced when you have at least two VLC players connected to the server and stop playing in one of them, but the probability of crash in this case is lower. This bug is also present in GStreamer 1.3.2, but does not lead to segfault. Instead, you will see the following: (rtsp_server:3048): CRITICAL **: gst_rtsp_session_touch: assertion 'GST_IS_RTSP_SESSION (session)' failed After some investigation it is clear that gst_rtsp_session_touch is called after the session has been completely destroyed. Here is some logs: 0:00:14.962651710 16272 0xcc090 INFO rtspclient rtsp-client.c:2288:client_session_removed: client 0xb2a28: session 0x46a88840 removed 0:00:14.967672252 16272 0xcc090 INFO rtspclient rtsp-client.c:332:client_unwatch_session: client 0xb2a28: unwatch session 0x46a88840 //// SESSION IS REMOVED HERE //// 0:00:14.967786752 16272 0xcc090 INFO rtspsession rtsp-session.c:149:gst_rtsp_session_finalize: finalize session 0x46a88840 0:00:14.968056586 16272 0xcc090 INFO rtspclient rtsp-client.c:3163:closed: client 0xb29a0: connection closed 0:00:14.968207211 16272 0xcc090 INFO rtspclient rtsp-client.c:3394:client_watch_notify: client 0xb29a0: watch destroyed 0:00:14.968312419 16272 0xcc090 DEBUG rtspserver rtsp-server.c:999:unmanage_client:<GstRTSPServer@0x1cac8> unmanage client 0xb29a0 0:00:14.968446627 16272 0xcc090 DEBUG rtspserver rtsp-server.c:979:free_client_context: free context 0xcb5b0 0:00:14.968559252 16272 0xcc090 DEBUG default rtsp-thread-pool.c:173:gst_rtsp_thread_stop: stop thread 0x16598 0:00:14.968640877 16272 0xcc090 INFO rtspclient rtsp-client.c:371:gst_rtsp_client_finalize: finalize client 0xb29a0 0:00:14.968892836 16272 0xcc090 INFO rtspmedia rtsp-media.c:2537:gst_rtsp_media_unprepare: media 0x4103d118 still prepared 1 times 0:00:14.969181669 16272 0x42015550 LOG adapter gstadapter.c:605:gst_adapter_flush_unchecked:<GstAdapter@0x4100d868> flushing out head buffer 0:00:14.969292377 16272 0x420154f0 DEBUG rtpsource rtpsource.c:1305:rtp_source_process_rb: got RB packet: SSRC 6d054002, FL 0, PL -1, HS 85145, jitter 8900, LSR 3e15:9cc3, DLSR 0000:172d 0:00:14.969416294 16272 0x420154f0 DEBUG rtpsource rtpsource.c:1332:rtp_source_process_rb: NTP 3e15:b71c, round trip 0000:032c 0:00:14.969532461 16272 0x420154f0 INFO rtspstream rtsp-stream.c:1431:on_ssrc_active: 0x410400e8: source 0x46a86a88 in transport 0x4100bc90 is active //// KEEP-ALIVE FOR REMOVED SESSION //// 0:00:14.969600377 16272 0x420154f0 INFO rtspclient rtsp-client.c:1303:do_keepalive: keep session 0x46a88840 alive Segmentation fault
Can you check if this is still a problem with 1.5.1 or GIT master? There were many changes. Also can you provide a backtrace of the crash?
Seems that 1.5.1 has this issue, too.
Created attachment 304831 [details] Segfault backtrace Full backtrace of crashed thread (session is touched after finalizing).
Looks like it should disconnect those signal handlers early enough, or maybe just shut down the rtpbin early enough. Depending on at which point this actually happens.
Created attachment 304840 [details] [review] Possible fix Please look at this possible patch (created for gst-rtsp-server 1.4.5). This helped for me, but this might be insufficient.
Created attachment 334826 [details] [review] Possible fix for 1.7.x and later I got the segfault with the same reason in 1.7.x. I've tried to update the version to fresher one commit by commit and I found that segfault disappearied after commit below: author Jake Foytik <jake.foytik@ipconfigure.com> 2016-04-25 12:55:25 (GMT) committer Sebastian Dröge <sebastian@centricular.com> 2016-04-29 08:49:14 (GMT) commit fe5f8077c1523206147c746cc40364ea16da669f (patch) tree 5db07f69e50cfa166d8b752c36ca0ed132ff4468 parent aa9a2443a1d303727167b5b253e09e31fea6f09b (diff) rtsp-stream: Fix crash on cleanup with shared media and multiple udpsrc - Unicast udpsrcs are now managed in a hash table. This allows for proper cleanup in with shared streams and fixes a memory leak. - Unicast udpsrcs are now properly cleaned up when shared connections exit. See the update_transport() function. - Create unit test for shared media. https://bugzilla.gnome.org/show_bug.cgi?id=764744 But there is no direct connection between this commit and segfault leaving so it looks like a race condition as Denis said. I've done some research and found that segfault happens if "gst_rtsp_stream_transport_keep_alive" calls after removing session in "client_unwatch_session" but before "gst_rtsp_stream_transport_finalize" function or "g_object_set_qdata (source, ssrc_stream_map_key, NULL)" function calls. To prevent it, I've written a patch to unset do_keepalive callback before the session will be removed, please watch it.
Review of attachment 334826 [details] [review]: Makes sense, just some minor things ::: gst/rtsp-server/rtsp-session.c @@ +251,3 @@ } +void gst_unset_transport_keepalive(GstRTSPSessionMedia * sessmedia) Please make this a static function, and namespace it properly (gst_rtsp_session_media_...) ::: gst/rtsp-server/rtsp-session.h @@ +107,3 @@ const gchar *path, GstRTSPMedia *media); +void gst_unset_transport_keepalive (GstRTSPSessionMedia * sessmedia); And don't put it in the header then, it's internal API
Created attachment 334827 [details] [review] possible fix 2 - refactoring
Comment on attachment 334827 [details] [review] possible fix 2 - refactoring Looks good, I'll merge this later. You would ideally also run gst-indent over this, and write a commit message in the format: one-line summary longer explanation in multiple lines https://bugzilla.gnome.org/show_bug.cgi?id=750544
commit 6136ef66d4333cc8cf17eb3b28f3746a0cf9346c Author: Kseniia <vasilchukkseniia@gmail.com> Date: Mon Sep 5 18:31:36 2016 +0300 rtsp-session: Fix segfault when doing keep-alive after removing the session If keep-alive happens after removing the session but before finalizing the stream transport, we would segfault. https://bugzilla.gnome.org/show_bug.cgi?id=750544