From 4a9df820e3311b609d6900b447cbc45b9263a6f1 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Mon, 31 Oct 2005 09:52:13 +0000 Subject: [PATCH] Boo! Original commit message from CVS: 2005-10-31 Andy Wingo * Boo! * gst/gstqueue.c (gst_queue_chain): Fix downstream leaky mode. * gst/gstobject.c (gst_object_dispatch_properties_changed): No need to serialize property notifications on GLib 2.8. GLib 2.6 has the possibility of deadlocks here if code calling notify() or set() has a lock that can be taken in another notify handler (ABBA with class lock and e.g. python GIL state lock). --- ChangeLog | 12 ++++++++++++ gst/gstobject.c | 6 ++++++ gst/gstqueue.c | 20 ++++++++------------ plugins/elements/gstqueue.c | 20 ++++++++------------ 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/ChangeLog b/ChangeLog index 659da8d831..57802b7327 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2005-10-31 Andy Wingo + + * Boo! + + * gst/gstqueue.c (gst_queue_chain): Fix downstream leaky mode. + + * gst/gstobject.c (gst_object_dispatch_properties_changed): No + need to serialize property notifications on GLib 2.8. GLib 2.6 has + the possibility of deadlocks here if code calling notify() or + set() has a lock that can be taken in another notify handler (ABBA + with class lock and e.g. python GIL state lock). + 2005-10-28 Julien MOUTTE * gst/gstbus.c: Doc updates. diff --git a/gst/gstobject.c b/gst/gstobject.c index c771f6c1c9..f0a60a19fb 100644 --- a/gst/gstobject.c +++ b/gst/gstobject.c @@ -575,7 +575,10 @@ gst_object_dispatch_properties_changed (GObject * object, klass = GST_OBJECT_GET_CLASS (object); +#ifndef GST_HAVE_GLIB_2_8 GST_CLASS_LOCK (klass); +#endif + /* do the standard dispatching */ PATCH_REFCOUNT (object); G_OBJECT_CLASS (parent_class)->dispatch_properties_changed (object, n_pspecs, @@ -617,7 +620,10 @@ gst_object_dispatch_properties_changed (GObject * object, gst_object_unref (old_parent); } g_free (name); + +#ifndef GST_HAVE_GLIB_2_8 GST_CLASS_UNLOCK (klass); +#endif } /** diff --git a/gst/gstqueue.c b/gst/gstqueue.c index 599b8491f4..1f47d800ea 100644 --- a/gst/gstqueue.c +++ b/gst/gstqueue.c @@ -653,7 +653,8 @@ gst_queue_chain (GstPad * pad, GstBuffer * buffer) GST_CAT_DEBUG_OBJECT (queue_dataflow, queue, "queue is full, leaking buffer on downstream end"); - for (item = queue->queue->head; item != NULL; item = item->next) { + for (item = g_queue_peek_head_link (queue->queue); item; + item = item->next) { if (GST_IS_BUFFER (item->data)) { leak = item->data; break; @@ -664,22 +665,17 @@ gst_queue_chain (GstPad * pad, GstBuffer * buffer) * in here. That cannot happen, since we had >= 1 bufs */ g_assert (leak); - /* Now remove it from the list, fixing up the GQueue - * CHECKME: is a queue->head the first or the last item? */ - item = g_list_delete_link (queue->queue->head, item); - queue->queue->head = g_list_first (item); - queue->queue->tail = g_list_last (item); - queue->queue->length--; + /* Now remove the link from the queue */ + g_queue_delete_link (queue->queue, item); /* and unref the buffer at the end. Twice, because we keep a ref * to make things read-only. Also keep our list uptodate. */ - queue->cur_level.bytes -= GST_BUFFER_SIZE (buffer); + queue->cur_level.bytes -= GST_BUFFER_SIZE (leak); queue->cur_level.buffers--; - if (GST_BUFFER_DURATION (buffer) != GST_CLOCK_TIME_NONE) - queue->cur_level.time -= GST_BUFFER_DURATION (buffer); + if (GST_BUFFER_DURATION (leak) != GST_CLOCK_TIME_NONE) + queue->cur_level.time -= GST_BUFFER_DURATION (leak); - gst_buffer_unref (buffer); - gst_buffer_unref (buffer); + gst_buffer_unref (leak); break; } diff --git a/plugins/elements/gstqueue.c b/plugins/elements/gstqueue.c index 599b8491f4..1f47d800ea 100644 --- a/plugins/elements/gstqueue.c +++ b/plugins/elements/gstqueue.c @@ -653,7 +653,8 @@ gst_queue_chain (GstPad * pad, GstBuffer * buffer) GST_CAT_DEBUG_OBJECT (queue_dataflow, queue, "queue is full, leaking buffer on downstream end"); - for (item = queue->queue->head; item != NULL; item = item->next) { + for (item = g_queue_peek_head_link (queue->queue); item; + item = item->next) { if (GST_IS_BUFFER (item->data)) { leak = item->data; break; @@ -664,22 +665,17 @@ gst_queue_chain (GstPad * pad, GstBuffer * buffer) * in here. That cannot happen, since we had >= 1 bufs */ g_assert (leak); - /* Now remove it from the list, fixing up the GQueue - * CHECKME: is a queue->head the first or the last item? */ - item = g_list_delete_link (queue->queue->head, item); - queue->queue->head = g_list_first (item); - queue->queue->tail = g_list_last (item); - queue->queue->length--; + /* Now remove the link from the queue */ + g_queue_delete_link (queue->queue, item); /* and unref the buffer at the end. Twice, because we keep a ref * to make things read-only. Also keep our list uptodate. */ - queue->cur_level.bytes -= GST_BUFFER_SIZE (buffer); + queue->cur_level.bytes -= GST_BUFFER_SIZE (leak); queue->cur_level.buffers--; - if (GST_BUFFER_DURATION (buffer) != GST_CLOCK_TIME_NONE) - queue->cur_level.time -= GST_BUFFER_DURATION (buffer); + if (GST_BUFFER_DURATION (leak) != GST_CLOCK_TIME_NONE) + queue->cur_level.time -= GST_BUFFER_DURATION (leak); - gst_buffer_unref (buffer); - gst_buffer_unref (buffer); + gst_buffer_unref (leak); break; }