g_object_new: check for NULL from _constructor()
authorRyan Lortie <desrt@desrt.ca>
Fri, 26 Apr 2013 15:27:51 +0000 (11:27 -0400)
committerRyan Lortie <desrt@desrt.ca>
Fri, 26 Apr 2013 15:34:27 +0000 (11:34 -0400)
There is some code in the wild (like in gnome-session) that does this
from its custom _constructor() implementation:

{
  GObject *obj;

  obj = ((chain up));

  if (!object_is_viable (obj))
    {
      g_object_unref (obj);
      return NULL;
    }
  else
    return obj;
}

This has never been a valid use of GObject and this code has always
caused memory to be leaked[1] by growing the construction_objects list.
The ability to legitimately return NULL from a constructor was exactly
the reason that we created GInitable, in fact.

That doesn't change the fact that the g_object_new() rewrite will crash
in this case, so instead of doing that, let's emit a critical and avoid
the crash.  This will allow people to upgrade their GLib without also
upgrading their gnome-session.  Meanwhile, people can fix their broken
code.

[1] not in the strictest sense of the word, because it's still reachable

gobject/gobject.c

index 543fc7d..38bbcca 100644 (file)
@@ -1661,6 +1661,18 @@ g_object_new_with_custom_constructor (GObjectClass          *class,
     g_value_unset (&cvalues[cvals_used]);
   g_free (cvalues);
 
+  /* There is code in the wild that relies on being able to return NULL
+   * from its custom constructor.  This was never a supported operation
+   * and will leak memory, but since the code is already out there...
+   */
+  if (object == NULL)
+    {
+      g_critical ("Custom constructor for class %s returned NULL (which is invalid).  Unable to remove object "
+                  "from construction_objects list, so memory was probably just leaked.  Please use GInitable "
+                  "instead.", G_OBJECT_CLASS_NAME (class));
+      return NULL;
+    }
+
   /* g_object_init() will have added us to the construction_objects
    * list.  Check if we're in it (and remove us) in order to find
    * out if we were newly-constructed or this is an already-existing