memory: check semantics of nested mappings

Count how many mappings are currently active and also with what access pattern.
Update the design doc with restrictions on the access patterns for nested
mappings.
Check if nested mappings obey the access mode restrictions of the design doc.
Add various unit tests to check the desired behaviour.

docs/design/part-memory.txt

@@ -115,7 +115,9 @@ Data Access
  performed. The call will update the new memory size with the specified size.
 
  It is allowed to map multiple times with different access modes. for each of
- the map calls, an corresponding unmap call needs to be made.
+ the map calls, an corresponding unmap call needs to be made. WRITE-only memory
+ cannot be mapped in READ mode and READ-only memory cannot be mapped in WRITE
+ mode.
  
  The memory pointer returned from the map call is guaranteed to remain valid in
  the requested mapping mode until the corresponding unmap call is performed on
@@ -127,7 +129,8 @@ Data Access
  When the final reference on a memory object is dropped, all outstanding
  mappings are automatically unmapped.
 
- Resizing a GstMemory does not influence any current mappings an any way.
+ Resizing a GstMemory does not influence any current mappings an any way. Note
+ however that the unmap call can resize the buffer again.
 
 
 Copy

gst/gstmemory.c

@@ -123,6 +123,7 @@ _default_mem_init (GstMemoryDefault * mem, GstMemoryFlags flags,
   mem->mem.flags = flags;
   mem->mem.refcount = 1;
   mem->mem.parent = parent ? gst_memory_ref (parent) : NULL;
+  mem->mem.state = 0;
   mem->slice_size = slice_size;
   mem->data = data;
   mem->free_func = free_func;
@@ -220,6 +221,10 @@ _default_mem_map (GstMemoryDefault * mem, gsize * size, gsize * maxsize,
 static gboolean
 _default_mem_unmap (GstMemoryDefault * mem, gpointer data, gsize size)
 {
+  GST_DEBUG ("mem: %p, data %p, size %u", mem, data, size);
+  GST_DEBUG ("mem: %p, data %p,  offset %u, size %u, maxsize %u", mem,
+      mem->data, mem->offset, mem->size, mem->maxsize);
+
   g_return_val_if_fail ((guint8 *) data >= mem->data
       && (guint8 *) data < mem->data + mem->maxsize, FALSE);
 
@@ -477,11 +482,43 @@ gpointer
 gst_memory_map (GstMemory * mem, gsize * size, gsize * maxsize,
     GstMapFlags flags)
 {
-  g_return_val_if_fail (mem != NULL, NULL);
-  g_return_val_if_fail (!(flags & GST_MAP_WRITE) ||
-      GST_MEMORY_IS_WRITABLE (mem), NULL);
+  gpointer res;
+  gint access_mode, state, newstate;
 
-  return mem->allocator->info.map (mem, size, maxsize, flags);
+  g_return_val_if_fail (mem != NULL, NULL);
+  access_mode = flags & 3;
+  g_return_val_if_fail (!(access_mode & GST_MAP_WRITE)
+      || GST_MEMORY_IS_WRITABLE (mem), NULL);
+
+  do {
+    state = g_atomic_int_get (&mem->state);
+    if (state == 0) {
+      /* nothing mapped, set access_mode and refcount */
+      newstate = 4 | access_mode;
+    } else {
+      /* access_mode must match */
+      g_return_val_if_fail ((state & access_mode) == access_mode, NULL);
+      /* increase refcount */
+      newstate = state + 4;
+    }
+  } while (!g_atomic_int_compare_and_exchange (&mem->state, state, newstate));
+
+  res = mem->allocator->info.map (mem, size, maxsize, flags);
+
+  if (G_UNLIKELY (res == NULL)) {
+    /* something went wrong, restore the orginal state again */
+    do {
+      state = g_atomic_int_get (&mem->state);
+      /* there must be a ref */
+      g_return_val_if_fail (state >= 4, NULL);
+      /* decrease the refcount */
+      newstate = state - 4;
+      /* last refcount, unset access_mode */
+      if (newstate < 4)
+        newstate = 0;
+    } while (!g_atomic_int_compare_and_exchange (&mem->state, state, newstate));
+  }
+  return res;
 }
 
 /**
@@ -502,9 +539,31 @@ gst_memory_map (GstMemory * mem, gsize * size, gsize * maxsize,
 gboolean
 gst_memory_unmap (GstMemory * mem, gpointer data, gssize size)
 {
+  gboolean need_unmap = TRUE;
+  gint state, newstate;
+
   g_return_val_if_fail (mem != NULL, FALSE);
 
-  return mem->allocator->info.unmap (mem, data, size);
+  do {
+    state = g_atomic_int_get (&mem->state);
+
+    /* there must be a ref */
+    g_return_val_if_fail (state >= 4, FALSE);
+
+    if (need_unmap) {
+      /* try to unmap, only do this once */
+      if (!mem->allocator->info.unmap (mem, data, size))
+        return FALSE;
+      need_unmap = FALSE;
+    }
+    /* success, try to decrease the refcount */
+    newstate = state - 4;
+    /* last refcount, unset access_mode */
+    if (newstate < 4)
+      newstate = 0;
+  } while (!g_atomic_int_compare_and_exchange (&mem->state, state, newstate));
+
+  return TRUE;
 }
 
 /**

gst/gstmemory.h

@@ -71,6 +71,7 @@ typedef enum {
  * @flags: memory flags
  * @refcount: refcount
  * @parent: parent memory block
+ * @state: private state
  *
  * Base structure for memory implementations. Custom memory will put this structure
  * as the first member of their structure.
@@ -81,6 +82,7 @@ struct _GstMemory {
   GstMemoryFlags  flags;
   gint            refcount;
   GstMemory      *parent;
+  volatile gint   state;
 };
 
 /**

tests/check/gst/gstmemory.c

@@ -463,7 +463,7 @@ GST_START_TEST (test_map_nested)
   fail_unless (data1 != NULL);
   fail_unless (size1 == 100);
 
-  data2 = gst_memory_map (mem, &size2, &maxsize2, GST_MAP_WRITE);
+  data2 = gst_memory_map (mem, &size2, &maxsize2, GST_MAP_READ);
   fail_unless (data2 == data1);
   fail_unless (size2 == 100);
 
@@ -471,6 +471,36 @@ GST_START_TEST (test_map_nested)
   gst_memory_unmap (mem, data1, size1);
   gst_memory_unmap (mem, data2, size2);
 
+  data1 = gst_memory_map (mem, &size1, &maxsize1, GST_MAP_READ);
+  /* not allowed */
+  ASSERT_CRITICAL (gst_memory_map (mem, &size2, &maxsize2, GST_MAP_WRITE));
+  ASSERT_CRITICAL (gst_memory_map (mem, &size2, &maxsize2, GST_MAP_READWRITE));
+  data2 = gst_memory_map (mem, &size2, &maxsize2, GST_MAP_READ);
+  gst_memory_unmap (mem, data1, size1);
+  gst_memory_unmap (mem, data2, size2);
+  fail_unless (mem->state == 0);
+
+  data1 = gst_memory_map (mem, &size1, &maxsize1, GST_MAP_WRITE);
+  /* not allowed */
+  ASSERT_CRITICAL (gst_memory_map (mem, &size2, &maxsize2, GST_MAP_READ));
+  ASSERT_CRITICAL (gst_memory_map (mem, &size2, &maxsize2, GST_MAP_READWRITE));
+  data2 = gst_memory_map (mem, &size2, &maxsize2, GST_MAP_WRITE);
+  gst_memory_unmap (mem, data1, size1);
+  gst_memory_unmap (mem, data2, size2);
+  /* nothing was mapped */
+  ASSERT_CRITICAL (gst_memory_unmap (mem, data2, size2));
+
+  data1 = gst_memory_map (mem, &size1, &maxsize1, GST_MAP_READWRITE);
+  data2 = gst_memory_map (mem, &size2, &maxsize2, GST_MAP_READ);
+  gst_memory_unmap (mem, data2, size2);
+  data2 = gst_memory_map (mem, &size2, &maxsize2, GST_MAP_WRITE);
+  gst_memory_unmap (mem, data2, size2);
+  /* ut of range */
+  ASSERT_CRITICAL (gst_memory_unmap (mem, (guint8 *) data1 - 1, size1));
+  gst_memory_unmap (mem, data1, size1);
+  /* nothing was mapped */
+  ASSERT_CRITICAL (gst_memory_unmap (mem, data1, size1));
+
   gst_memory_unref (mem);
 }