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

@@ -1,3 +1,29 @@
+2005-09-19  Thomas Vander Stichele  <thomas at apestaart dot org>
+
+	* 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  <wim@fluendo.com>
 
 	* check/gst/gstbin.c: (pop_messages), (GST_START_TEST):

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

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;
 }

common

@@ -1 +1 @@
-Subproject commit eac450a673cd1d3a606fe75399d0cd1fcb1d0d7b
+Subproject commit 13022c3cb4558d201e2ddf3e65d2e36b16eedc4a

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");
 }
 
 /**

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!");

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);

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 =

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;
   }

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) {

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

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;
 }