[Bug 760551] New: Deadlock in shmsink

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=760551

            Bug ID: 760551
           Summary: Deadlock in shmsink
    Classification: Platform
           Product: GStreamer
           Version: 1.2.4
                OS: Linux
            Status: NEW
          Severity: major
          Priority: Normal
         Component: gst-plugins-bad
          Assignee: gstreamer-bugs at lists.freedesktop.org
          Reporter: mattcrane at tycoint.com
        QA Contact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---

I discovered a deadlock during some stress testing of our video streaming
architecture that utilizes shmsink/shmsrc to stream video buffers between two
processes.  The system in question was running with GStreamer 1.2.4, the
version that ships with SuSE Linux Enterprise 12. A quick examination of git
seems to indicate that the problematic code still exists in the current version
of gst-plugins-bad.

In our tests, a shmsrc client repeatedly connects/disconnects at various
intervals to receive video from the shmsink server pipeline.  At some point
during the test, the shmsink element in the server process was observed to
deadlock in gst_shm_sink_allocator_free().  The stack trace of the appropriate
thread from gdb:

#0  __lll_lock_wait () at
../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x00007f96de0334bf in _L_lock_1081 () from /lib64/libpthread.so.0
#2  0x00007f96de033438 in __GI___pthread_mutex_lock (mutex=0x7f96d7636cd0) at
../nptl/pthread_mutex_lock.c:134
#3  0x00007f96e00b4011 in g_mutex_lock () from /usr/lib64/libglib-2.0.so.0
#4  0x00007f96d13ef25b in gst_shm_sink_allocator_free (allocator=<optimized
out>, mem=0x7f96c402ce10)
    at gstshmsink.c:175
#5  0x00007f96df4d3f9d in _gst_memory_free (mem=0x7f96c402ce10) at
gstmemory.c:97
#6  0x00007f96d13efd6b in gst_memory_unref (memory=0x7f96c402ce10)
    at /usr/include/gstreamer-1.0/gst/gstmemory.h:325
#7  gst_shm_sink_render (bsink=<optimized out>, buf=0x7f96c40b48f0) at
gstshmsink.c:725
#8  0x00007f96df7a12ea in gst_base_sink_chain_unlocked
(basesink=basesink at entry=0x7f96d7638200,
    obj=obj at entry=0x7f96c40b48f0, is_list=is_list at entry=0, pad=<optimized out>)
at gstbasesink.c:3378
#9  0x00007f96df7a2854 in gst_base_sink_chain_main (basesink=0x7f96d7638200,
pad=<optimized out>,
    obj=0x7f96c40b48f0, is_list=0) at gstbasesink.c:3486
#10 0x00007f96df4d8c58 in gst_pad_chain_data_unchecked (data=0x7f96c40b48f0,
type=4112, pad=0x7f96d7629a60)
    at gstpad.c:3760
#11 gst_pad_push_data (pad=pad at entry=0x7f96d7629830, type=type at entry=4112,
data=<optimized out>)
    at gstpad.c:3990
#12 0x00007f96df4df906 in gst_pad_push (pad=pad at entry=0x7f96d7629830,
buffer=<optimized out>) at gstpad.c:4093
#13 0x00007f96df7a79a5 in gst_base_src_loop (pad=0x7f96d7629830) at
gstbasesrc.c:2779
#14 0x00007f96df5064c9 in gst_task_func (task=0x7f96d7615cb0) at gsttask.c:316
#15 0x00007f96e00997cc in ?? () from /usr/lib64/libglib-2.0.so.0
#16 0x00007f96e0098e15 in ?? () from /usr/lib64/libglib-2.0.so.0
#17 0x00007f96de0310a4 in start_thread (arg=0x7f96977fe700) at
pthread_create.c:309
#18 0x00007f96de32b7fd in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

The lock in question is the shmsink GST_OBJECT lock.  A closer examination in
gdb revealed that the same thread already held a lock on the mutex.  Looking at
the code, we found the initial GST_OBJECT_LOCK() being acquired at the start of
gst_shm_sink_render(), but it isn't released until *after* a call to
gst_memory_unref():

lines 714-721 in gstshmsink.c (exact line #s may vary depending on version):

    while (self->wait_for_connection && !self->clients) {
      g_cond_wait (&self->cond, GST_OBJECT_GET_LOCK (self));
      if (self->unlock) {
        gst_memory_unref (memory);
        GST_OBJECT_UNLOCK (self);
        return GST_FLOW_FLUSHING;
      }
    }

As can be seen in the stack trace, this gst_memory_unref() leads to calls to
_gst_memory_free() and gst_shm_sink_allocator_free().  Inside of
gst_shm_sink_allocator_free(), the code tries to acquire the shmsink
GST_OBJECT_LOCK() again:

static void
gst_shm_sink_allocator_free (GstAllocator * allocator, GstMemory * mem)
{
  GstShmSinkMemory *mymem = (GstShmSinkMemory *) mem;

  if (mymem->block) {
    GST_OBJECT_LOCK (mymem->sink);
    sp_writer_free_block (mymem->block);
    GST_OBJECT_UNLOCK (mymem->sink);
    gst_object_unref (mymem->sink);
  }
  gst_object_unref (mem->allocator);

  g_slice_free (GstShmSinkMemory, mymem);
}


I *think* this deadlock can be resolved by moving the gst_memory_unref() call
in gst_shm_sink_render() to after the GST_OBJECT_UNLOCK() call, as in the
following diff:

@@ -714,8 +722,8 @@
     while (self->wait_for_connection && !self->clients) {
       g_cond_wait (&self->cond, GST_OBJECT_GET_LOCK (self));
       if (self->unlock) {
-        gst_memory_unref (memory);
         GST_OBJECT_UNLOCK (self);
+        gst_memory_unref (memory);
         return GST_FLOW_FLUSHING;
       }
     }

We are currently testing this change in our own rebuilt version of the plugin
and wanted to raise this issue with the GStreamer team to make sure they were
aware of the problem and to get feedback on whether the proposed fix was
appropriate.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.