memory: gst_memory_share may fail to exclusively lock the parent memory

Now that locking exclusively dows not always succeed, we need to signal
the failure case from gst_memory_init.

Rather than introducing an API or funcionality change to gst_memory_init,
workaround by checking exclusivity in the calling code.

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

diff --git a/gst/gstmemory.c b/gst/gstmemory.c
index 3505ac2..5da0f58 100644 (file)
--- a/gst/gstmemory.c
+++ b/gst/gstmemory.c
@@ -123,6 +123,7 @@ gst_memory_init (GstMemory * mem, GstMemoryFlags flags,
 
   mem->allocator = gst_object_ref (allocator);
   if (parent) {
+    /* FIXME 2.0: this can fail if the memory is already write locked */
     gst_memory_lock (parent, GST_LOCK_FLAG_EXCLUSIVE);
     gst_memory_ref (parent);
   }
@@ -387,8 +388,28 @@ gst_memory_share (GstMemory * mem, gssize offset, gssize size)
   g_return_val_if_fail (!GST_MEMORY_FLAG_IS_SET (mem, GST_MEMORY_FLAG_NO_SHARE),
       NULL);
 
+  /* whether we can lock the memory exclusively */
+  /* in order to maintain backwards compatibility by not requiring subclasses
+   * to lock the memory themselves and propagate the possible failure in their
+   * mem_share implementation */
+  /* FIXME 2.0: remove and fix gst_memory_init() and/or all memory subclasses
+   * to propagate this failure case */
+  if (!gst_memory_lock (mem, GST_LOCK_FLAG_EXCLUSIVE))
+    return NULL;
+
+  /* double lock to ensure we are not mapped writable without an
+   * exclusive lock. */
+  if (!gst_memory_lock (mem, GST_LOCK_FLAG_EXCLUSIVE)) {
+    gst_memory_unlock (mem, GST_LOCK_FLAG_EXCLUSIVE);
+    return NULL;
+  }
+
   shared = mem->allocator->mem_share (mem, offset, size);
 
+  /* unlocking before calling the subclass would be racy */
+  gst_memory_unlock (mem, GST_LOCK_FLAG_EXCLUSIVE);
+  gst_memory_unlock (mem, GST_LOCK_FLAG_EXCLUSIVE);
+
   return shared;
 }
 

diff --git a/tests/check/gst/gstmemory.c b/tests/check/gst/gstmemory.c
index d1964da..6600e12 100644 (file)
--- a/tests/check/gst/gstmemory.c
+++ b/tests/check/gst/gstmemory.c
@@ -89,6 +89,13 @@ GST_START_TEST (test_submemory)
   gst_memory_unref (sub);
 
   gst_memory_unmap (memory, &info);
+
+  /* test write map + share failure */
+  fail_unless (gst_memory_map (memory, &info, GST_MAP_WRITE));
+  sub = gst_memory_share (memory, 0, 4);
+  fail_unless (sub == NULL, "share with a write map succeeded");
+
+  gst_memory_unmap (memory, &info);
   gst_memory_unref (memory);
 }