miniobject: avoid race in bufferpool release

Avoid playing with the refcount to decide when a buffer has been recycled by the
dispose function. The problem is that we then temporarily can have a buffer with
a refcount > 1 being acquired from the pool, which is not writable. Instead use
a simple boolean return value from the dispose function to inform the called
that the object was recycled or not.

gst/gstbuffer.c

@@ -363,18 +363,22 @@ _gst_buffer_copy (GstBuffer * buffer)
 
 /* the default dispose function revives the buffer and returns it to the
  * pool when there is a pool */
-static void
+static gboolean
 _gst_buffer_dispose (GstBuffer * buffer)
 {
   GstBufferPool *pool;
 
-  if ((pool = buffer->pool) != NULL) {
+  /* no pool, do free */
-    /* keep the buffer alive */
+  if ((pool = buffer->pool) == NULL)
-    gst_buffer_ref (buffer);
+    return TRUE;
-    /* return the buffer to the pool */
+
-    GST_CAT_LOG (GST_CAT_BUFFER, "release %p to pool %p", buffer, pool);
+  /* keep the buffer alive */
-    gst_buffer_pool_release_buffer (pool, buffer);
+  gst_buffer_ref (buffer);
-  }
+  /* return the buffer to the pool */
+  GST_CAT_LOG (GST_CAT_BUFFER, "release %p to pool %p", buffer, pool);
+  gst_buffer_pool_release_buffer (pool, buffer);
+
+  return FALSE;
 }
 
 static void

gst/gstminiobject.c

@@ -254,17 +254,16 @@ gst_mini_object_unref (GstMiniObject * mini_object)
       GST_MINI_OBJECT_REFCOUNT_VALUE (mini_object) - 1);
 
   if (G_UNLIKELY (g_atomic_int_dec_and_test (&mini_object->refcount))) {
-    /* At this point, the refcount of the object is 0. We increase the refcount
+    gboolean do_free;
-     * here because if a subclass recycles the object and gives out a new
-     * reference we don't want to free the instance anymore. */
-    gst_mini_object_ref (mini_object);
 
     if (mini_object->dispose)
-      mini_object->dispose (mini_object);
+      do_free = mini_object->dispose (mini_object);
+    else
+      do_free = TRUE;
 
-    /* decrement the refcount again, if the subclass recycled the object we don't
+    /* if the subclass recycled the object (and returned FALSE) we don't
      * want to free the instance anymore */
-    if (G_LIKELY (g_atomic_int_dec_and_test (&mini_object->refcount))) {
+    if (G_LIKELY (do_free)) {
       /* The weak reference stack is freed in the notification function */
       if (mini_object->n_weak_refs)
         weak_refs_notify (mini_object);

gst/gstminiobject.h

@@ -52,10 +52,12 @@ typedef GstMiniObject * (*GstMiniObjectCopyFunction) (const GstMiniObject *obj);
  * Function prototype for when a miniobject has lost its last refcount.
  * Implementation of the mini object are allowed to revive the
  * passed object by doing a gst_mini_object_ref(). If the object is not
- * revived after the dispose function, the memory associated with the
+ * revived after the dispose function, the function should return %TRUE
- * object is freed.
+ * and the memory associated with the object is freed.
+ *
+ * Returns: %TRUE if the object should be cleaned up.
  */
-typedef void (*GstMiniObjectDisposeFunction) (GstMiniObject *obj);
+typedef gboolean (*GstMiniObjectDisposeFunction) (GstMiniObject *obj);
 /**
  * GstMiniObjectFreeFunction:
  * @obj: MiniObject to free