GNOME Bugzilla – Bug 308766
gst_element_factory_create() may invalidate pad templates for the factory it is called with
Last modified: 2005-06-23 13:29:08 UTC
when calling gst_element_factory_create(factory), the factory object itself may be altered, for example, its pad templates may be freed and re-allocated. this can cause a problem with applications storing the templates earlier. for example, the examples/manual/dynamic.c sample program does this: for (pads = gst_element_factory_get_pad_templates (factory); pads != NULL; pads = pads->next) { GstPadTemplate *templ = GST_PAD_TEMPLATE (pads->data); ... element = gst_element_factory_create (factory, NULL); close_link (pad, element, templ->name_template, gst_element_factory_get_pad_templates (factory)); in the close_link() call, templ and templ->name_template may be invalid (already) freed, and thus may access invalid memory. the reason behind the possible change is that gst_element_factory_create() may result in the invocation of gst_element_register(), which might reuse the factory storage, and clean up the old factory resources, in gst/gstelementfactory.c: factory = gst_element_factory_find (name); if (!factory) { factory = GST_ELEMENT_FACTORY (g_object_new (GST_TYPE_ELEMENT_FACTORY, NULL)); gst_plugin_feature_set_name (GST_PLUGIN_FEATURE (factory), name); GST_LOG_OBJECT (factory, "Created new elementfactory for type %s", g_type_name (type)); gst_plugin_add_feature (plugin, GST_PLUGIN_FEATURE (factory)); } else { g_return_val_if_fail (factory->type == 0, FALSE); gst_element_factory_cleanup (factory); GST_LOG_OBJECT (factory, "Reuse existing elementfactory for type %s", g_type_name (type)); } where gst_element_factory_cleanup() will free up the pad templates associated with the factory.
Created attachment 48208 [details] [review] patch for gstreamer to produce debug output relevant to this bug
Created attachment 48209 [details] the original dynamic sample, with some debug output
Created attachment 48210 [details] the result of running gstreamer with the above debug output added to achieve the above output, compile gstreamer with the above debug output patch, and the dynamic_orig.c sample with the debug output. then run: valgrind --tool=memcheck --leak-check=yes --num-callers=40 --leak-resolution=high ./dynamic_orig <sample mp3 file> > valgrind_result 2>&1
the important part of the debug output is: gst_element_factory_cleanup #1, factory: id3demuxbin, padtemplates: 1C14FF74 template: 1C187868, name: 1C1879A0, src template: 1C187E08, name: 1C187F40, sink gst_pad_template_dispose #1, template: 1C187868, name: 1C1879A0, src gst_pad_template_dispose #1, template: 1C187E08, name: 1C187F40, sink ... ==31063== ==31063== Invalid read of size 4 ==31063== at 0x80497C9: try_to_plug (dynamic_orig.c:205) ==31063== by 0x80498D1: cb_typefound (dynamic_orig.c:237) ==31063== by 0x1B96B8CB: gst_marshal_VOID__UINT_BOXED (gstmarshal.c:470) ==31063== by 0x1BA50C4C: g_closure_invoke (in /usr/lib/libgobject-2.0.so.0.600.3) ==31063== Address 0x1C187E34 is 44 bytes inside a block of size 76 free'd ==31063== at 0x1B90657D: free (vg_replace_malloc.c:153) ==31063== by 0x1BCA9502: g_free (in /usr/lib/libglib-2.0.so.0.600.3) ==31063== ... try_to_plug #1.2, template: 1C187E08, name: 1C187F40, sink it's easy to see that during the call to gst_element_factory_create(), for some reason the very factory is reaused that is passed to the call, and the template referenced as tmpl in try_to_plug (1C187E08) is freed in the process.
Created attachment 48213 [details] [review] a patch solving the issue it turns out the dynamic sample was wrong, here's a patch to fix it
I'd also suggest adding explanation to the documentation of gst_element_factory_create(), that it might invalidate properties of the factory it's working on. I thing it would be important, as I myself have spent 3 days trying to nail down this issue, hopefully other poeple won't have to.
Created attachment 48214 [details] [review] documentation patch for the gst_element_factory_create() function this patch documents the fact that gst_element_factory_create() may invalidate properties of the factory it works on.
Both look good, thanks. Applied.