mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2025-02-18 20:25:25 +00:00
Code Issues 1 Projects Releases Wiki Activity

fakesink: hack around crasher bug in g_object_notify() for out-of-band events

Browse source 
GObject may crash if two threads do concurrent g_object_notify() on the same
object. This may happen if fakesink receives an out-of-band event such as
FLUSH_START while processing a buffer or serialised event in the streaming
thread. Since this may happen with the default settings during a common
operation like a seek, and there seems to be little chance of a timely fix
in GObject (see #166020), we should hack around this issue by protecting all
of fakesink's direct g_object_notify() calls with a lock.

Also add unit test for the above.

Fixes #554460.
This commit is contained in:
Tim-Philipp Müller 2009-05-30 20:36:25 +01:00
parent 7c4e618471
commit 7e4b164c12
4 changed files with 138 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
plugins/elements/gstfakesink.c
View file

@ -113,6 +113,7 @@ static void gst_fake_sink_set_property (GObject * object, guint prop_id,
const GValue * value, GParamSpec * pspec);
static void gst_fake_sink_get_property (GObject * object, guint prop_id,
GValue * value, GParamSpec * pspec);
static void gst_fake_sink_finalize (GObject * obj);
static GstStateChangeReturn gst_fake_sink_change_state (GstElement * element,
GstStateChange transition);
@ -182,6 +183,7 @@ gst_fake_sink_class_init (GstFakeSinkClass * klass)
gobject_class->set_property = GST_DEBUG_FUNCPTR (gst_fake_sink_set_property);
gobject_class->get_property = GST_DEBUG_FUNCPTR (gst_fake_sink_get_property);
gobject_class->finalize = gst_fake_sink_finalize;
g_object_class_install_property (gobject_class, PROP_STATE_ERROR,
g_param_spec_enum ("state-error", "State Error",
@ -265,6 +267,17 @@ gst_fake_sink_init (GstFakeSink * fakesink, GstFakeSinkClass * g_class)
fakesink->state_error = DEFAULT_STATE_ERROR;
fakesink->signal_handoffs = DEFAULT_SIGNAL_HANDOFFS;
fakesink->num_buffers = DEFAULT_NUM_BUFFERS;
g_static_rec_mutex_init (&fakesink->notify_lock);
}
static void
gst_fake_sink_finalize (GObject * obj)
{
GstFakeSink *sink = GST_FAKE_SINK (obj);
g_static_rec_mutex_free (&sink->notify_lock);
G_OBJECT_CLASS (parent_class)->finalize (obj);
}
static void
@ -344,6 +357,20 @@ gst_fake_sink_get_property (GObject * object, guint prop_id, GValue * value,
}
}
static void
gst_fake_sink_notify_last_message (GstFakeSink * sink)
{
/* FIXME: this hacks around a bug in GLib/GObject: doing concurrent
* g_object_notify() on the same object might lead to crashes, see
* http://bugzilla.gnome.org/show_bug.cgi?id=166020#c60 and follow-ups.
* So we really don't want to do a g_object_notify() here for out-of-band
* events with the streaming thread possibly also doing a g_object_notify()
* for an in-band buffer or event. */
g_static_rec_mutex_lock (&sink->notify_lock);
g_object_notify ((GObject *) sink, "last_message");
g_static_rec_mutex_unlock (&sink->notify_lock);
}
static gboolean
gst_fake_sink_event (GstBaseSink * bsink, GstEvent * event)
{
@ -367,7 +394,7 @@ gst_fake_sink_event (GstBaseSink * bsink, GstEvent * event)
g_free (sstr);
GST_OBJECT_UNLOCK (sink);
g_object_notify (G_OBJECT (sink), "last_message");
gst_fake_sink_notify_last_message (sink);
}
if (GST_BASE_SINK_CLASS (parent_class)->event) {
@ -392,7 +419,7 @@ gst_fake_sink_preroll (GstBaseSink * bsink, GstBuffer * buffer)
sink->last_message = g_strdup_printf ("preroll ******* ");
GST_OBJECT_UNLOCK (sink);
g_object_notify (G_OBJECT (sink), "last_message");
gst_fake_sink_notify_last_message (sink);
}
if (sink->signal_handoffs) {
g_signal_emit (sink,
@ -448,7 +475,7 @@ gst_fake_sink_render (GstBaseSink * bsink, GstBuffer * buf)
GST_MINI_OBJECT_CAST (buf)->flags, buf);
GST_OBJECT_UNLOCK (sink);
g_object_notify (G_OBJECT (sink), "last_message");
gst_fake_sink_notify_last_message (sink);
}
if (sink->signal_handoffs)
g_signal_emit (G_OBJECT (sink), gst_fake_sink_signals[SIGNAL_HANDOFF], 0,

1
plugins/elements/gstfakesink.h
View file

@ -82,6 +82,7 @@ struct _GstFakeSink {
gchar *last_message;
gint num_buffers;
gint num_buffers_left;
GStaticRecMutex notify_lock;
};
struct _GstFakeSinkClass {

4
tests/check/Makefile.am
View file

@ -152,6 +152,10 @@ libs_gdp_LDADD = \
elements_fdsrc_CFLAGS=$(GST_OBJ_CFLAGS) $(CHECK_CFLAGS) -DTESTFILE=\"$(top_srcdir)/configure.ac\"
elements_filesrc_CFLAGS=$(GST_OBJ_CFLAGS) $(CHECK_CFLAGS) -DTESTFILE=\"$(top_srcdir)/configure.ac\"
elements_fakesink_LDADD = \
$(top_builddir)/libs/gst/base/libgstbase-@GST_MAJORMINOR@.la \
$(LDADD)
libs_basesrc_LDADD = \
$(top_builddir)/libs/gst/base/libgstbase-@GST_MAJORMINOR@.la \
$(LDADD)

103
tests/check/elements/fakesink.c
View file

@ -4,6 +4,7 @@
*
* Copyright (C) <2005> Thomas Vander Stichele <thomas at apestaart dot org>
* <2007> Wim Taymans <wim@fluendo.com>
* <2009> Tim-Philipp Müller <tim centricular net>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@ -23,6 +24,7 @@
#include <unistd.h>
#include <gst/base/gstpushsrc.h>
#include <gst/check/gstcheck.h>
typedef struct
@ -858,6 +860,106 @@ GST_START_TEST (test_position)
GST_END_TEST;
/* like fakesrc, but also pushes an OOB event after each buffer */
typedef GstPushSrc OOBSource;
typedef GstPushSrcClass OOBSourceClass;
GST_BOILERPLATE (OOBSource, oob_source, GstPushSrc, GST_TYPE_PUSH_SRC);
static void
oob_source_base_init (gpointer g_class)
{
static GstStaticPadTemplate sinktemplate = GST_STATIC_PAD_TEMPLATE ("src",
GST_PAD_SRC, GST_PAD_ALWAYS, GST_STATIC_CAPS_ANY);
gst_element_class_add_pad_template (GST_ELEMENT_CLASS (g_class),
gst_static_pad_template_get (&sinktemplate));
}
static GstFlowReturn
oob_source_create (GstPushSrc * src, GstBuffer ** p_buf)
{
*p_buf = gst_buffer_new ();
gst_pad_push_event (GST_BASE_SRC_PAD (src),
gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM_OOB, NULL));
return GST_FLOW_OK;
}
static void
oob_source_class_init (OOBSourceClass * klass)
{
GstPushSrcClass *pushsrc_class = GST_PUSH_SRC_CLASS (klass);
pushsrc_class->create = GST_DEBUG_FUNCPTR (oob_source_create);
}
static void
oob_source_init (OOBSource * src, OOBSourceClass * g_class)
{
/* nothing to do */
}
#define NOTIFY_RACE_NUM_PIPELINES 20
typedef struct
{
GstElement *src;
GstElement *queue;
GstElement *sink;
GstElement *pipe;
} NotifyRacePipeline;
static void
test_notify_race_setup_pipeline (NotifyRacePipeline * p)
{
p->pipe = gst_pipeline_new ("pipeline");
p->src = g_object_new (oob_source_get_type (), NULL);
p->queue = gst_element_factory_make ("queue", NULL);
g_object_set (p->queue, "max-size-buffers", 2, NULL);
p->sink = gst_element_factory_make ("fakesink", NULL);
gst_bin_add (GST_BIN (p->pipe), p->src);
gst_bin_add (GST_BIN (p->pipe), p->queue);
gst_bin_add (GST_BIN (p->pipe), p->sink);
gst_element_link_many (p->src, p->queue, p->sink, NULL);
fail_unless_equals_int (gst_element_set_state (p->pipe, GST_STATE_PLAYING),
GST_STATE_CHANGE_ASYNC);
fail_unless_equals_int (gst_element_get_state (p->pipe, NULL, NULL, -1),
GST_STATE_CHANGE_SUCCESS);
}
static void
test_notify_race_cleanup_pipeline (NotifyRacePipeline * p)
{
gst_element_set_state (p->pipe, GST_STATE_NULL);
gst_object_unref (p->pipe);
memset (p, 0, sizeof (NotifyRacePipeline));
}
/* we create N pipelines to make sure the notify race isn't per-class, but
* only per instance */
GST_START_TEST (test_notify_race)
{
NotifyRacePipeline pipelines[NOTIFY_RACE_NUM_PIPELINES];
int i;
for (i = 0; i < G_N_ELEMENTS (pipelines); ++i) {
test_notify_race_setup_pipeline (&pipelines[i]);
}
g_usleep (2 * G_USEC_PER_SEC);
for (i = 0; i < G_N_ELEMENTS (pipelines); ++i) {
test_notify_race_cleanup_pipeline (&pipelines[i]);
}
}
GST_END_TEST;
static Suite *
fakesink_suite (void)
{
@ -872,6 +974,7 @@ fakesink_suite (void)
tcase_add_test (tc_chain, test_eos);
tcase_add_test (tc_chain, test_eos2);
tcase_add_test (tc_chain, test_position);
tcase_add_test (tc_chain, test_notify_race);
return s;
}