sys/: Implement a locking order to ensure we always take the object lock before the...

Original commit message from CVS:
* sys/ximage/ximagesink.c: (gst_ximagesink_ximage_new),
(gst_ximagesink_change_state):
* sys/xvimage/xvimagesink.c: (gst_xvimagesink_xvimage_new),
(gst_xvimagesink_change_state):
Implement a locking order to ensure we always take the object lock
before the x_lock and never vice-versa.

diff --git a/ChangeLog b/ChangeLog
index 4575a53..fc2a329 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2006-07-13  Jan Schmidt  <thaytan@mad.scientist.com>
 
+       * sys/ximage/ximagesink.c: (gst_ximagesink_ximage_new),
+       (gst_ximagesink_change_state):
+       * sys/xvimage/xvimagesink.c: (gst_xvimagesink_xvimage_new),
+       (gst_xvimagesink_change_state):
+       Implement a locking order to ensure we always take the object lock
+       before the x_lock and never vice-versa.
+
+2006-07-13  Jan Schmidt  <thaytan@mad.scientist.com>
+
        * gst/playback/gstdecodebin.c: (find_compatibles):
        Fix a caps leak when linking (#347304)
 

diff --git a/sys/ximage/ximagesink.c b/sys/ximage/ximagesink.c
index 1600736..c3bb5db 100644 (file)
--- a/sys/ximage/ximagesink.c
+++ b/sys/ximage/ximagesink.c
@@ -428,6 +428,7 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, GstCaps * caps)
         ximagesink->xcontext->depth,
         ZPixmap, NULL, &ximage->SHMInfo, ximage->width, ximage->height);
     if (!ximage->ximage) {
+      g_mutex_unlock (ximagesink->x_lock);
       GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               ximage->width, ximage->height),
@@ -444,6 +445,7 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, GstCaps * caps)
     ximage->SHMInfo.shmid = shmget (IPC_PRIVATE, ximage->size,
         IPC_CREAT | 0777);
     if (ximage->SHMInfo.shmid == -1) {
+      g_mutex_unlock (ximagesink->x_lock);
       GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               ximage->width, ximage->height),
@@ -453,6 +455,7 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, GstCaps * caps)
 
     ximage->SHMInfo.shmaddr = shmat (ximage->SHMInfo.shmid, 0, 0);
     if (ximage->SHMInfo.shmaddr == ((void *) -1)) {
+      g_mutex_unlock (ximagesink->x_lock);
       GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               ximage->width, ximage->height),
@@ -471,6 +474,7 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, GstCaps * caps)
     ximage->SHMInfo.readOnly = FALSE;
 
     if (XShmAttach (ximagesink->xcontext->disp, &ximage->SHMInfo) == 0) {
+      g_mutex_unlock (ximagesink->x_lock);
       GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               ximage->width, ximage->height), ("Failed to XShmAttach"));
@@ -487,6 +491,7 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, GstCaps * caps)
         ZPixmap, 0, NULL,
         ximage->width, ximage->height, ximagesink->xcontext->bpp, 0);
     if (!ximage->ximage) {
+      g_mutex_unlock (ximagesink->x_lock);
       GST_ELEMENT_ERROR (ximagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               ximage->width, ximage->height),
@@ -509,9 +514,8 @@ gst_ximagesink_ximage_new (GstXImageSink * ximagesink, GstCaps * caps)
   /* Keep a ref to our sink */
   ximage->ximagesink = gst_object_ref (ximagesink);
 
-beach:
   g_mutex_unlock (ximagesink->x_lock);
-
+beach:
   if (!succeeded) {
     gst_ximage_buffer_free (ximage);
     ximage = NULL;
@@ -1352,17 +1356,21 @@ gst_ximagesink_change_state (GstElement * element, GstStateChange transition)
 {
   GstXImageSink *ximagesink;
   GstStateChangeReturn ret = GST_STATE_CHANGE_SUCCESS;
+  GstXContext *xcontext = NULL;
 
   ximagesink = GST_XIMAGESINK (element);
 
   switch (transition) {
     case GST_STATE_CHANGE_NULL_TO_READY:
-      GST_OBJECT_LOCK (ximagesink);
-      ximagesink->running = TRUE;
 
       /* Initializing the XContext */
       if (!ximagesink->xcontext)
-        ximagesink->xcontext = gst_ximagesink_xcontext_get (ximagesink);
+        xcontext = gst_ximagesink_xcontext_get (ximagesink);
+
+      GST_OBJECT_LOCK (ximagesink);
+      ximagesink->running = TRUE;
+      if (xcontext)
+        ximagesink->xcontext = xcontext;
       GST_OBJECT_UNLOCK (ximagesink);
 
       if (!ximagesink->xcontext) {

diff --git a/sys/xvimage/xvimagesink.c b/sys/xvimage/xvimagesink.c
index 4044029..472e80f 100644 (file)
--- a/sys/xvimage/xvimagesink.c
+++ b/sys/xvimage/xvimagesink.c
@@ -536,12 +536,13 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, GstCaps * caps)
         xvimage->im_format, NULL,
         xvimage->width, xvimage->height, &xvimage->SHMInfo);
     if (!xvimage->xvimage) {
+      g_mutex_unlock (xvimagesink->x_lock);
       GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               xvimage->width, xvimage->height),
           ("could not XvShmCreateImage a %dx%d image",
               xvimage->width, xvimage->height));
-      goto beach;
+      goto beach_unlocked;
     }
 
     /* we have to use the returned data_size for our shm size */
@@ -551,22 +552,24 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, GstCaps * caps)
     xvimage->SHMInfo.shmid = shmget (IPC_PRIVATE, xvimage->size,
         IPC_CREAT | 0777);
     if (xvimage->SHMInfo.shmid == -1) {
+      g_mutex_unlock (xvimagesink->x_lock);
       GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               xvimage->width, xvimage->height),
           ("could not get shared memory of %d bytes", xvimage->size));
-      goto beach;
+      goto beach_unlocked;
     }
 
     xvimage->SHMInfo.shmaddr = shmat (xvimage->SHMInfo.shmid, 0, 0);
     if (xvimage->SHMInfo.shmaddr == ((void *) -1)) {
+      g_mutex_unlock (xvimagesink->x_lock);
       GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               xvimage->width, xvimage->height),
           ("Failed to shmat: %s", g_strerror (errno)));
       /* Clean up the shared memory segment */
       shmctl (xvimage->SHMInfo.shmid, IPC_RMID, 0);
-      goto beach;
+      goto beach_unlocked;
     }
 
     /* Delete the shared memory segment as soon as we manage to attach.
@@ -578,10 +581,11 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, GstCaps * caps)
     xvimage->SHMInfo.readOnly = FALSE;
 
     if (XShmAttach (xvimagesink->xcontext->disp, &xvimage->SHMInfo) == 0) {
+      g_mutex_unlock (xvimagesink->x_lock);
       GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE,
           ("Failed to create output image buffer of %dx%d pixels",
               xvimage->width, xvimage->height), ("Failed to XShmAttach"));
-      goto beach;
+      goto beach_unlocked;
     }
 
     XSync (xvimagesink->xcontext->disp, FALSE);
@@ -594,12 +598,13 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, GstCaps * caps)
         xvimagesink->xcontext->xv_port_id,
         xvimage->im_format, NULL, xvimage->width, xvimage->height);
     if (!xvimage->xvimage) {
+      g_mutex_unlock (xvimagesink->x_lock);
       GST_ELEMENT_ERROR (xvimagesink, RESOURCE, WRITE,
           ("Failed to create outputimage buffer of %dx%d pixels",
               xvimage->width, xvimage->height),
           ("could not XvCreateImage a %dx%d image",
               xvimage->width, xvimage->height));
-      goto beach;
+      goto beach_unlocked;
     }
 
     /* we have to use the returned data_size for our image size */
@@ -613,9 +618,6 @@ gst_xvimagesink_xvimage_new (GstXvImageSink * xvimagesink, GstCaps * caps)
   GST_BUFFER_DATA (xvimage) = (guchar *) xvimage->xvimage->data;
   GST_BUFFER_SIZE (xvimage) = xvimage->size;
 
-beach:
-  g_mutex_unlock (xvimagesink->x_lock);
-
 beach_unlocked:
   if (!succeeded) {
     gst_xvimage_buffer_free (xvimage);
@@ -1852,20 +1854,23 @@ gst_xvimagesink_change_state (GstElement * element, GstStateChange transition)
 {
   GstStateChangeReturn ret = GST_STATE_CHANGE_SUCCESS;
   GstXvImageSink *xvimagesink;
+  GstXContext *xcontext = NULL;
 
   xvimagesink = GST_XVIMAGESINK (element);
 
   switch (transition) {
     case GST_STATE_CHANGE_NULL_TO_READY:
-      GST_OBJECT_LOCK (xvimagesink);
-      xvimagesink->running = TRUE;
       /* Initializing the XContext */
-      if (!xvimagesink->xcontext &&
-          !(xvimagesink->xcontext =
-              gst_xvimagesink_xcontext_get (xvimagesink))) {
-        GST_OBJECT_UNLOCK (xvimagesink);
-        return GST_STATE_CHANGE_FAILURE;
+      if (xvimagesink->xcontext == NULL) {
+        xcontext = gst_xvimagesink_xcontext_get (xvimagesink);
+        if (xcontext == NULL)
+          return GST_STATE_CHANGE_FAILURE;
       }
+
+      GST_OBJECT_LOCK (xvimagesink);
+      xvimagesink->running = TRUE;
+      if (xcontext)
+        xvimagesink->xcontext = xcontext;
       GST_OBJECT_UNLOCK (xvimagesink);
 
       /* update object's par with calculated one if not set yet */