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 735569 - rtspconnection: Crash due to no protection of watchs readsrc
rtspconnection: Crash due to no protection of watchs readsrc
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other Linux
: Normal normal
: 1.4.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-08-28 07:20 UTC by Göran Jönsson
Modified: 2014-09-04 09:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for gst-plugins-base (3.08 KB, patch)
2014-08-28 07:20 UTC, Göran Jönsson
needs-work Details | Review
new gst-plugins-base patch (2.61 KB, patch)
2014-08-29 04:52 UTC, Göran Jönsson
committed Details | Review

Description Göran Jönsson 2014-08-28 07:20:41 UTC
Created attachment 284662 [details] [review]
patch for gst-plugins-base

This happends due to it is possible to manipulate watchs readsrc from different threads.

This is happening when running with multiple client threads in rtsp server.
More specific from handle_tunnel when calling 
gst_rtsp_watch_reset (opriv->watch);
( Observe that opriv is from another client saved in hash_table )
( Observe same watch Thread4 #4 watch=0x58ced8  Thread12 #11 watch=0x58ced8) 

Thread 12 (Thread 21856)

  • #0 g_mutex_get_impl
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gthread-posix.c line 123
  • #1 g_mutex_lock
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gthread-posix.c line 210
  • #2 g_source_destroy_internal
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 1185
  • #3 closure_invoke_notifiers
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/gobject/gclosure.c line 264
  • #4 g_closure_invalidate
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/gobject/gclosure.c line 562
  • #5 g_closure_unref
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/gobject/gclosure.c line 593
  • #6 g_source_destroy_internal
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 1204
  • #7 g_child_source_remove_internal
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 1446
  • #8 g_source_destroy_internal
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 1222
  • #9 g_child_source_remove_internal
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 1446
  • #10 g_source_remove_child_source
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 1479
  • #11 gst_rtsp_source_dispatch_read
    at /home/goranjn/FLASHDIR/560aftersemester/libs/gst-plugins-base/gst-plugins-base/gst-libs/gst/rtsp/gstrtspconnection.c line 3056
  • #12 g_main_dispatch
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 3063
  • #13 g_main_context_dispatch
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 3639
  • #14 g_main_context_iterate
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 3710
  • #15 g_main_loop_run
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gmain.c line 3904
  • #16 do_loop
    at /home/goranjn/FLASHDIR/560aftersemester/libs/gst-rtsp-server/gst-rtsp-server/gst/rtsp-server/rtsp-thread-pool.c line 329
  • #17 g_thread_pool_thread_proxy
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gthreadpool.c line 309
  • #18 g_thread_proxy
    at /home/goranjn/FLASHDIR/560aftersemester/libs/glib/glib/glib/gthread.c line 798
  • #19 start_thread
    at pthread_create.c line 310
  • #20 __thread_start
    from target/mipsisa32r2el-axis-linux-gnu/lib/libc.so.6

Comment 1 Göran Jönsson 2014-08-28 07:25:31 UTC
The trace contain two threads. If I just check the "+" sign i only see one.
But if I choose to open link in new tab i can see both.
Comment 2 Sebastian Dröge (slomo) 2014-08-28 07:54:32 UTC
Review of attachment 284662 [details] [review]:

The same seems to be true for the writesrc.

::: gst-libs/gst/rtsp/gstrtspconnection.c
@@ +2935,3 @@
   guint id;
   GMutex mutex;
+  GMutex readsrc_mutex;

Why not just reuse the existing mutex?

@@ +3413,2 @@
   if (watch->conn->input_stream) {
+    g_mutex_lock (&watch->readsrc_mutex);

Just put the lock/unlock outside the if/else block
Comment 3 Göran Jönsson 2014-08-29 04:52:25 UTC
Created attachment 284788 [details] [review]
new gst-plugins-base patch

New patch with corrections.
Comment 4 Sebastian Dröge (slomo) 2014-08-29 08:29:41 UTC
commit acdb7feacf5253b661b0c0e5ce5283cb85483849
Author: Göran Jönsson <goranjn@axis.com>
Date:   Wed Aug 27 13:45:57 2014 +0200

    rtspconnection: Protect readsrc, writesrc and controllsrc with a mutex
    
    Fixes a crash when controlsrc, readsrc or writesrc are modified from
    gst_rtsp_source_dispatch_read/write and gst_rtsp_watch_reset at the
    same time.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=735569