CODEWITHSUNDEEP
cmemory-leaksglibgobject
geoffjay
geoffjay

Reputation: 413

JSON (GLib) serialization leaks memory for a GHashTable

I'm trying to serialize and deserialize a GHashTable in and out of JSON and valgrind reports memory that is definitely lost as a result of doing this. There are suppressions for g_hash_table_new in the glib.suppressions file that they distribute, but none for g_hash_table_new_full which is what I see a bunch of. My hash table is a GObject property that has been set using g_param_spec_pointer, and the function to serialize it is:

static JsonNode *
foo_obj_serialize_property (JsonSerializable *serializable,
                            const gchar      *name,
                            const GValue     *value,
                            GParamSpec       *pspec)
{
  JsonNode *retval = NULL;

  if (g_strcmp0 (name, "list") == 0)
    {
      GHashTable *list = NULL;
      GHashTableIter iter;
      JsonArray *arr = NULL;
      gpointer key, val;

      retval = json_node_new (JSON_NODE_ARRAY);

      g_return_val_if_fail (value != NULL, retval);
      g_return_val_if_fail (G_VALUE_HOLDS_POINTER (value), retval);

      list = g_value_get_pointer (value);

      arr = json_array_new ();

      if (list != NULL)
        {
          g_hash_table_iter_init (&iter, list);
          while (g_hash_table_iter_next (&iter, &key, &val))
            {
              JsonNode *node = NULL;
              JsonObject *obj = NULL;
              FooItem *item;

              item = FOO_ITEM (val);
              node = json_gobject_serialize (G_OBJECT (item));

              if (JSON_NODE_HOLDS_OBJECT (node))
                {
                  obj = json_node_get_object (node);
                  json_array_add_object_element (arr, obj);
                }
            }
        }

      json_node_take_array (retval, arr);
    }

  return retval;
}

the deserialize is:

static gboolean
foo_obj_deserialize_property (JsonSerializable *serializable,
                              const gchar      *name,
                              GValue           *value,
                              GParamSpec       *pspec,
                              JsonNode         *property_node)
{
  gboolean retval = FALSE;

  if (g_strcmp0 (name, "list") == 0)
    {
      GHashTable *list;
      JsonArray *arr;

      arr = json_node_get_array (property_node);
      list = g_hash_table_new_full (g_str_hash,
                                    g_str_equal,
                                    g_free,
                                    NULL);

      for (gint i = 0; i < json_array_get_length (arr); i++)
        {
          g_autoptr (FooItem) item = NULL;
          JsonNode *node = NULL;

          node = json_array_get_element (arr, i);

          item = FOO_ITEM (json_gobject_deserialize (FOO_TYPE_ITEM, node));
          g_return_val_if_fail (FOO_IS_ITEM (item), FALSE);
          g_object_ref (item);
          g_hash_table_insert (list,
                               g_strdup (foo_item_get_key (item)),
                               item);
        }

      g_value_set_pointer (value, list);

      retval = TRUE;
    }

  return retval;
}

my GObject class property get/set functions are:

static void
foo_obj_get_property (GObject    *object,
                      guint       prop_id,
                      GValue     *value,
                      GParamSpec *pspec)
{
  FooObj *self = FOO_OBJ (object);

  switch (prop_id)
    {
    case PROP_LIST:
      g_value_set_pointer (value, self->list);
      break;

    default:
      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
    }
}

static void
foo_obj_set_property (GObject      *object,
                      guint         prop_id,
                      const GValue *value,
                      GParamSpec   *pspec)
{
  FooObj *self = FOO_OBJ (object);

  switch (prop_id)
    {
    case PROP_LIST:
      foo_obj_set_list (self, g_value_get_pointer (value));
      break;

    default:
      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
    }
}

and finally the class getter/setter:

GHashTable *
foo_obj_get_list (FooObj *self)
{
  GHashTable *list;

  g_return_val_if_fail (FOO_IS_OBJ (self), NULL);

  g_object_get (self, "list", &list, NULL);

  return list;
}

void
foo_obj_set_list (FooObj     *self,
                  GHashTable *list)
{
  g_return_if_fail (FOO_IS_OBJ (self));

  if (self->list == list)
    return;

  if (list)
    g_hash_table_ref (list);

  if (self->list)
    g_hash_table_unref (self->list);

  self->list = list;

  g_object_notify_by_pspec (G_OBJECT (self), properties [PROP_LIST]);
}

I'm running a memory check using valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --leak-resolution=high --num-callers=20 --suppressions=../tests/glib.supp foo, and I get messages for lost memory for serialize showing:

==24303== 32 bytes in 1 blocks are definitely lost in loss record 1,029 of 1,852
==24303==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==24303==    by 0x494CAB1: g_malloc (in /usr/lib/libglib-2.0.so.0.6000.4)
==24303==    by 0x492C8E4: g_slice_alloc (in /usr/lib/libglib-2.0.so.0.6000.4)
==24303==    by 0x4933839: g_slice_alloc0 (in /usr/lib/libglib-2.0.so.0.6000.4)
==24303==    by 0x4D72CEE: json_node_alloc (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4D72D37: json_node_new (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4D77B37: json_gobject_serialize (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4867AC9: foo_item_serialize_property (foo-item.c:71)
==24303==    by 0x4D779C1: ??? (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4D77B42: json_gobject_serialize (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4861681: foo_obj_serialize_property (foo-obj.c:146)
==24303==    by 0x4D779C1: ??? (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4D77B42: json_gobject_serialize (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4D77B82: json_gobject_to_data (in /usr/lib/libjson-glib-1.0.so.0.400.4)

and for deserialize:

==24303== 184 (88 direct, 96 indirect) bytes in 1 blocks are definitely lost in loss record 1,774 of 1,852
==24303==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==24303==    by 0x494CAB1: g_malloc (in /usr/lib/libglib-2.0.so.0.6000.4)
==24303==    by 0x492C8E4: g_slice_alloc (in /usr/lib/libglib-2.0.so.0.6000.4)
==24303==    by 0x4966C5E: g_hash_table_new_full (in /usr/lib/libglib-2.0.so.0.6000.4)
==24303==    by 0x4861AB2: foo_obj_deserialize_property (foo-obj.c:264)
==24303==    by 0x4D77F00: ??? (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4D7826B: json_gobject_from_data (in /usr/lib/libjson-glib-1.0.so.0.400.4)
==24303==    by 0x4862BEE: foo_obj_deserialize (foo-obj.c:472)

I'm having a hard time seeing what I've missed, and various attempts to clear pointers result in a double free or error about ref count == 0. I know there's a lot of GLib suppressions because of how it handles memory but I don't know if these fall under that.

I could probably box the hash table and simplify serialization by registering a function with json-glib, but I don't want to go that direction unless I need to.

Upvotes: 1

Views: 472

Answers (1)

Alexander Dmitriev
Alexander Dmitriev

Reputation: 2525

node = json_gobject_serialize (G_OBJECT (item));

After this line you do nothing with node, neither store it somewhere, nor free. This results in leaking memory. Also note, that JsonNode is a GBoxed, not GObject. Use json-node-free to free it.

I'd also suggest you read about reference counting and analysing valgrind output.

The stack trace tells you where the leaked memory was allocated. Memcheck cannot tell you why the memory leaked, unfortunately.

For deserialize method it was allocated at (foo-obj.c:264), that's g_hash_table_new_full. You create a hash table, but don't destroy it.

Upvotes: 2

Related Questions