From e9c15d53212ff424a047e933ca9ab662ae155b56 Mon Sep 17 00:00:00 2001
From: Matthew Waters <matthew@centricular.com>
Date: Tue, 2 Jun 2015 16:14:50 +1000
Subject: [PATCH] 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
---
 gst/gstmemory.c             | 21 +++++++++++++++++++++
 tests/check/gst/gstmemory.c |  7 +++++++
 2 files changed, 28 insertions(+)

diff --git a/gst/gstmemory.c b/gst/gstmemory.c
index 3505ac2795..5da0f58729 100644
--- 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 d1964da3ca..6600e1227c 100644
--- a/tests/check/gst/gstmemory.c
+++ b/tests/check/gst/gstmemory.c
@@ -88,6 +88,13 @@ GST_START_TEST (test_submemory)
   /* clean up */
   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);
 }