From 91cc8f969e8aac952dcfa6c356a09cf191ec774e Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Mon, 19 Sep 2005 14:09:54 +0000 Subject: [PATCH] check/Makefile.am: re-enable tests now that leaks are plugged Original commit message from CVS: * check/Makefile.am: re-enable tests now that leaks are plugged * check/gst/gst.c: * check/gst/gstbin.c: * check/gst/gstpipeline.c: add some more tests while fixing leaks * common/check.mak: make sure binaries are uptodate when valgrinding/gdbing * gst/gst.c: * gst/gstelementfactory.c: remove a ref too many, and add a FIXME for when we get round to disposing of classes * gst/gstplugin.c: fix the refcounting when loading a plugin from a file and the code pretends that the pointer is the same even though of course it can change * gst/gstpluginfeature.c: unref plugins marked cached (a bit confusing as a name) as the docs state should be done various doc additions to explain refcounting * gst/gstregistry.c: * gst/gstregistryxml.c: debugging --- ChangeLog | 26 ++++++++++++++++++++++++++ check/Makefile.am | 8 +------- check/gst/gst.c | 24 ++++++++++++++++++++++++ common | 2 +- gst/gst.c | 3 +++ gst/gstelementfactory.c | 10 ++++++++-- gst/gstplugin.c | 17 +++++++++++------ gst/gstpluginfeature.c | 3 +++ gst/gstregistry.c | 37 +++++++++++++++++++++++++++---------- gst/gstregistryxml.c | 12 +++++++++--- tests/check/Makefile.am | 8 +------- tests/check/gst/gst.c | 24 ++++++++++++++++++++++++ 12 files changed, 138 insertions(+), 36 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1cb2c0b..321fcf5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,29 @@ +2005-09-19 Thomas Vander Stichele + + * check/Makefile.am: + re-enable tests now that leaks are plugged + * check/gst/gst.c: + * check/gst/gstbin.c: + * check/gst/gstpipeline.c: + add some more tests while fixing leaks + * common/check.mak: + make sure binaries are uptodate when valgrinding/gdbing + * gst/gst.c: + * gst/gstelementfactory.c: + remove a ref too many, and add a FIXME for when we get + round to disposing of classes + * gst/gstplugin.c: + fix the refcounting when loading a plugin from a file and + the code pretends that the pointer is the same even though + of course it can change + * gst/gstpluginfeature.c: + unref plugins marked cached (a bit confusing as a name) + as the docs state should be done + various doc additions to explain refcounting + * gst/gstregistry.c: + * gst/gstregistryxml.c: + debugging + 2005-09-19 Wim Taymans * check/gst/gstbin.c: (pop_messages), (GST_START_TEST): diff --git a/check/Makefile.am b/check/Makefile.am index 5798e9a..5e93d36 100644 --- a/check/Makefile.am +++ b/check/Makefile.am @@ -78,15 +78,9 @@ gst_libs_controller_LDADD = $(GST_OBJ_LIBS) \ # valgrind testing # these just need valgrind fixing, period VALGRIND_TO_FIX = \ - gst/gstbin \ - gst/gstelement \ - gst/gstevent \ - gst/gstghostpad \ - gst/gstpipeline \ elements/fakesrc \ - elements/identity \ + gst/gstevent \ generic/states \ - states/sinks \ gst-libs/controller \ pipelines/cleanup \ pipelines/simple_launch_lines diff --git a/check/gst/gst.c b/check/gst/gst.c index f839677..76e37ce 100644 --- a/check/gst/gst.c +++ b/check/gst/gst.c @@ -54,6 +54,28 @@ GST_START_TEST (test_deinit_sysclock) GST_END_TEST; +/* tests if we can create an element from a compiled-in plugin */ +GST_START_TEST (test_new_pipeline) +{ + GstElement *pipeline; + + pipeline = gst_pipeline_new ("pipeline"); + gst_object_unref (pipeline); +} + +GST_END_TEST; + +/* tests if we can load an element from a plugin */ +GST_START_TEST (test_new_fakesrc) +{ + GstElement *element; + + element = gst_element_factory_make ("fakesrc", NULL); + gst_object_unref (element); +} + +GST_END_TEST; + Suite * gst_suite (void) @@ -65,6 +87,8 @@ gst_suite (void) tcase_add_test (tc_chain, test_init); tcase_add_test (tc_chain, test_deinit); tcase_add_test (tc_chain, test_deinit_sysclock); + tcase_add_test (tc_chain, test_new_pipeline); + tcase_add_test (tc_chain, test_new_fakesrc); return s; } diff --git a/common b/common index eac450a..13022c3 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit eac450a673cd1d3a606fe75399d0cd1fcb1d0d7b +Subproject commit 13022c3cb4558d201e2ddf3e65d2e36b16eedc4a diff --git a/gst/gst.c b/gst/gst.c index 76ddfdf..9bcffe0 100644 --- a/gst/gst.c +++ b/gst/gst.c @@ -689,6 +689,7 @@ init_post (void) registry_file = g_build_filename (g_get_home_dir (), ".gstreamer-0.9", "registry.xml", NULL); } + GST_DEBUG ("Reading registry cache"); gst_registry_xml_read_cache (default_registry, registry_file); plugin_path = g_getenv ("GST_PLUGIN_SYSTEM_PATH"); @@ -913,6 +914,7 @@ gst_deinit (void) { GstClock *clock; + GST_INFO ("deinitializing GStreamer"); clock = gst_system_clock_obtain (); gst_object_unref (clock); gst_object_unref (clock); @@ -920,6 +922,7 @@ gst_deinit (void) gst_registry_deinit (); gst_initialized = FALSE; + GST_INFO ("deinitialized GStreamer"); } /** diff --git a/gst/gstelementfactory.c b/gst/gstelementfactory.c index ac404d6..5afb15cd 100644 --- a/gst/gstelementfactory.c +++ b/gst/gstelementfactory.c @@ -152,7 +152,8 @@ gst_element_factory_finalize (GObject * object) * gst_element_factory_find: * @name: name of factory to find * - * Search for an element factory of the given name. + * Search for an element factory of the given name. Refs the returned + * element factory; caller is responsible for unreffing. * * Returns: #GstElementFactory if found, NULL otherwise */ @@ -351,11 +352,11 @@ gst_element_factory_create (GstElementFactory * factory, const gchar * name) g_return_val_if_fail (factory != NULL, NULL); - gst_object_ref (factory); factory = GST_ELEMENT_FACTORY (gst_plugin_feature_load (GST_PLUGIN_FEATURE (factory))); if (factory == NULL) { + GST_DEBUG ("warning: loading the plugin for this factory returned NULL"); return NULL; } @@ -373,6 +374,10 @@ gst_element_factory_create (GstElementFactory * factory, const gchar * name) } #endif + /* FIXME: the object class gets a pointer to the factory that might + * be disposed at the end of this call if it was newly loaded; + * to fix that, we should ref and then unref in an object class finalize, + * which we don't have currently. */ oclass = GST_ELEMENT_CLASS (g_type_class_ref (factory->type)); if (oclass->elementfactory == NULL) oclass->elementfactory = factory; @@ -421,6 +426,7 @@ gst_element_factory_make (const gchar * factoryname, const gchar * name) GST_INFO ("no such element factory \"%s\"!", factoryname); return NULL; } + GST_LOG ("gstelementfactory: found factory %p", factory); element = gst_element_factory_create (factory, name); if (element == NULL) { GST_INFO_OBJECT (factory, "couldn't create instance!"); diff --git a/gst/gstplugin.c b/gst/gstplugin.c index 1cec9bb..ae5ca7e 100644 --- a/gst/gstplugin.c +++ b/gst/gstplugin.c @@ -324,7 +324,7 @@ GStaticMutex gst_plugin_loading_mutex = G_STATIC_MUTEX_INIT; * @filename: the plugin filename to load * @error: pointer to a NULL-valued GError * - * Loads the given plugin. + * Loads the given plugin and refs it. Caller needs to unref after use. * * Returns: a new GstPlugin or NULL, if an error occurred. */ @@ -816,25 +816,30 @@ gst_plugin_find_feature_by_name (GstPlugin * plugin, const gchar * name) * gst_plugin_load_by_name: * @name: name of plugin to load * - * Load the named plugin. + * Load the named plugin. Refs the plugin. * * Returns: whether the plugin was loaded or not */ GstPlugin * gst_plugin_load_by_name (const gchar * name) { - GstPlugin *plugin; + GstPlugin *plugin, *newplugin; GError *error = NULL; + GST_DEBUG ("looking up plugin %s in default registry", name); plugin = gst_registry_find_plugin (gst_registry_get_default (), name); if (plugin) { - plugin = gst_plugin_load_file (plugin->filename, &error); - if (!plugin) { + GST_DEBUG ("loading plugin %s from file %s", name, plugin->filename); + newplugin = gst_plugin_load_file (plugin->filename, &error); + gst_object_unref (plugin); + + if (!newplugin) { GST_WARNING ("load_plugin error: %s\n", error->message); g_error_free (error); return NULL; } - return plugin; + /* newplugin was reffed by load_file */ + return newplugin; } GST_DEBUG ("Could not find plugin %s in registry", name); diff --git a/gst/gstpluginfeature.c b/gst/gstpluginfeature.c index 04f74a1..fc6079c 100644 --- a/gst/gstpluginfeature.c +++ b/gst/gstpluginfeature.c @@ -83,15 +83,18 @@ gst_plugin_feature_load (GstPluginFeature * feature) g_return_val_if_fail (feature != NULL, FALSE); g_return_val_if_fail (GST_IS_PLUGIN_FEATURE (feature), FALSE); + GST_DEBUG ("loading plugin for feature %p", feature); if (feature->loaded) return feature; + GST_DEBUG ("loading plugin %s", feature->plugin_name); plugin = gst_plugin_load_by_name (feature->plugin_name); if (!plugin) { g_critical ("Failed to load plugin containing feature '%s'.", GST_PLUGIN_FEATURE_NAME (feature)); return NULL; } + GST_DEBUG ("loaded plugin %s", feature->plugin_name); gst_object_unref (plugin); real_feature = diff --git a/gst/gstregistry.c b/gst/gstregistry.c index d9bcdf5..4e419c8 100644 --- a/gst/gstregistry.c +++ b/gst/gstregistry.c @@ -190,8 +190,6 @@ gst_registry_finalize (GObject * object) GstPluginFeature *feature = f->data; if (feature) { - GST_DEBUG_OBJECT (registry, "removing feature %s", - gst_plugin_feature_get_name (feature)); gst_registry_remove_feature (registry, feature); } f = g_list_next (f); @@ -287,13 +285,14 @@ gst_registry_add_plugin (GstRegistry * registry, GstPlugin * plugin) GST_LOCK (registry); existing_plugin = gst_registry_lookup_locked (registry, plugin->filename); if (existing_plugin) { - GST_DEBUG ("Replacing existing plugin %p for filename \"%s\"", - existing_plugin, plugin->filename); + GST_DEBUG + ("Replacing existing plugin %p with new plugin %p for filename \"%s\"", + existing_plugin, plugin, plugin->filename); registry->plugins = g_list_remove (registry->plugins, existing_plugin); gst_object_unref (existing_plugin); } - GST_DEBUG ("Adding plugin %p for filename \"%s\"", plugin, plugin->filename); + GST_DEBUG ("adding plugin %p for filename \"%s\"", plugin, plugin->filename); registry->plugins = g_list_prepend (registry->plugins, plugin); @@ -349,13 +348,13 @@ gst_registry_add_feature (GstRegistry * registry, GstPluginFeature * feature) existing_feature = gst_registry_lookup_feature_locked (registry, feature->name); if (existing_feature) { - GST_DEBUG ("Replacing existing feature %p (%s)", + GST_DEBUG_OBJECT (registry, "Replacing existing feature %p (%s)", existing_feature, feature->name); registry->features = g_list_remove (registry->features, existing_feature); gst_object_unref (existing_feature); } - GST_DEBUG ("Adding feature %p (%s)", feature, feature->name); + GST_DEBUG_OBJECT (registry, "adding feature %p (%s)", feature, feature->name); registry->features = g_list_prepend (registry->features, feature); @@ -363,7 +362,7 @@ gst_registry_add_feature (GstRegistry * registry, GstPluginFeature * feature) gst_object_sink (feature); GST_UNLOCK (registry); - GST_DEBUG ("emitting feature-added for %s", feature->name); + GST_DEBUG_OBJECT (registry, "emitting feature-added for %s", feature->name); g_signal_emit (G_OBJECT (registry), gst_registry_signals[FEATURE_ADDED], 0, feature); @@ -381,6 +380,8 @@ void gst_registry_remove_feature (GstRegistry * registry, GstPluginFeature * feature) { g_return_if_fail (GST_IS_REGISTRY (registry)); + GST_DEBUG_OBJECT (registry, "removing feature %p (%s)", + feature, gst_plugin_feature_get_name (feature)); GST_LOCK (registry); registry->features = g_list_remove (registry->features, feature); @@ -398,8 +399,10 @@ gst_registry_remove_feature (GstRegistry * registry, GstPluginFeature * feature) * Runs a filter against all plugins in the registry and returns a GList with * the results. If the first flag is set, only the first match is * returned (as a list with a single object). + * Every plugin is reffed; use gst_plugin_list_free() after use, which + * will unref again. * - * Returns: a GList of plugins, gst_plugin_list_free after use. + * Returns: a #GList of #GstPlugin */ GList * gst_registry_plugin_filter (GstRegistry * registry, @@ -461,6 +464,7 @@ gst_registry_feature_filter (GstRegistry * registry, * @name: the plugin name to find * * Find the plugin with the given name in the registry. + * The plugin will be reffed; caller is responsible for unreffing. * * Returns: The plugin with the given name or NULL if the plugin was not found. */ @@ -605,6 +609,16 @@ gst_registry_lookup_locked (GstRegistry * registry, const char *filename) return NULL; } +/** + * gst_registry_lookup: + * @registry: the registry to look up in + * @filename: the name of the file to look up + * + * Look up a plugin in the given registry with the given filename. + * If found, plugin is reffed. Caller must unref after use. + * + * Returns: the #GstPlugin if found, or NULL if not. + */ GstPlugin * gst_registry_lookup (GstRegistry * registry, const char *filename) { @@ -721,13 +735,16 @@ _gst_registry_remove_cache_plugins (GstRegistry * registry) GList *g_next; GstPlugin *plugin; + GST_DEBUG_OBJECT (registry, "removing cached plugins"); g = registry->plugins; while (g) { g_next = g->next; plugin = g->data; if (plugin->flags & GST_PLUGIN_FLAG_CACHED) { + GST_DEBUG_OBJECT (registry, "removing cached plugin %s", + plugin->filename); registry->plugins = g_list_remove (registry->plugins, plugin); - //gst_object_unref (plugin); + gst_object_unref (plugin); } g = g_next; } diff --git a/gst/gstregistryxml.c b/gst/gstregistryxml.c index b1393b3..852dfd2 100644 --- a/gst/gstregistryxml.c +++ b/gst/gstregistryxml.c @@ -508,6 +508,8 @@ load_feature (xmlTextReaderPtr reader) GstPluginFeature *feature; GType type; + GST_DEBUG ("loading feature"); + if (!feature_name) return NULL; type = g_type_from_name (feature_name); @@ -526,8 +528,10 @@ load_feature (xmlTextReaderPtr reader) return NULL; } while ((ret = xmlTextReaderRead (reader)) == 1) { - if (xmlTextReaderDepth (reader) == depth) + if (xmlTextReaderDepth (reader) == depth) { + GST_DEBUG ("loaded feature %p with name %s", feature, feature->name); return feature; + } if (xmlTextReaderNodeType (reader) == XML_READER_TYPE_ELEMENT && xmlTextReaderDepth (reader) == depth + 1) { const gchar *tag = (gchar *) xmlTextReaderConstName (reader); @@ -618,11 +622,13 @@ static GstPlugin * load_plugin (xmlTextReaderPtr reader, GList ** feature_list) { int ret; - GstPlugin *plugin = g_object_new (GST_TYPE_PLUGIN, NULL); + GstPlugin *plugin; *feature_list = NULL; - GST_DEBUG ("parsing plugin"); + GST_DEBUG ("creating new plugin and parsing"); + + plugin = g_object_new (GST_TYPE_PLUGIN, NULL); plugin->flags |= GST_PLUGIN_FLAG_CACHED; while ((ret = xmlTextReaderRead (reader)) == 1) { diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 5798e9a..5e93d36 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -78,15 +78,9 @@ gst_libs_controller_LDADD = $(GST_OBJ_LIBS) \ # valgrind testing # these just need valgrind fixing, period VALGRIND_TO_FIX = \ - gst/gstbin \ - gst/gstelement \ - gst/gstevent \ - gst/gstghostpad \ - gst/gstpipeline \ elements/fakesrc \ - elements/identity \ + gst/gstevent \ generic/states \ - states/sinks \ gst-libs/controller \ pipelines/cleanup \ pipelines/simple_launch_lines diff --git a/tests/check/gst/gst.c b/tests/check/gst/gst.c index f839677..76e37ce 100644 --- a/tests/check/gst/gst.c +++ b/tests/check/gst/gst.c @@ -54,6 +54,28 @@ GST_START_TEST (test_deinit_sysclock) GST_END_TEST; +/* tests if we can create an element from a compiled-in plugin */ +GST_START_TEST (test_new_pipeline) +{ + GstElement *pipeline; + + pipeline = gst_pipeline_new ("pipeline"); + gst_object_unref (pipeline); +} + +GST_END_TEST; + +/* tests if we can load an element from a plugin */ +GST_START_TEST (test_new_fakesrc) +{ + GstElement *element; + + element = gst_element_factory_make ("fakesrc", NULL); + gst_object_unref (element); +} + +GST_END_TEST; + Suite * gst_suite (void) @@ -65,6 +87,8 @@ gst_suite (void) tcase_add_test (tc_chain, test_init); tcase_add_test (tc_chain, test_deinit); tcase_add_test (tc_chain, test_deinit_sysclock); + tcase_add_test (tc_chain, test_new_pipeline); + tcase_add_test (tc_chain, test_new_fakesrc); return s; } -- 2.7.4