From 63f8294c453df0c39eee60288e161676dfaec9c2 Mon Sep 17 00:00:00 2001
From: Will Thompson <will@willthompson.co.uk>
Date: Thu, 20 Sep 2018 16:56:26 +0100
Subject: [PATCH] gkeyfile: remain usable after g_key_file_free()

Previously, in the case where 'kf' has more than one ref, calling
g_key_file_free(kf) would break it. For example, calling
g_key_file_has_key(kf, ...) would hit the following assertion:

    g_hash_table_lookup: assertion 'hash_table != NULL' failed

This is because g_key_file_free() calls g_key_file_clear() which sets
self->groups and other fields to NULL; most lookup functions assume
these fields are non-NULL.

One fix would be to call g_key_file_init() right after
g_key_file_clear() in g_key_file_free(). However, in the case where
there are no other refs to the keyfile, this would mean allocating
many new hash tables which will be immediately destroyed when
g_key_file_unref() removes the last ref. Instead, inline the unref, and
re-initialize the internal state when the keyfile is still alive.
---
 glib/gkeyfile.c      |  6 +++++-
 glib/tests/keyfile.c | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/glib/gkeyfile.c b/glib/gkeyfile.c
index ae3bbbc1d..4e9b53541 100644
--- a/glib/gkeyfile.c
+++ b/glib/gkeyfile.c
@@ -1191,7 +1191,11 @@ g_key_file_free (GKeyFile *key_file)
   g_return_if_fail (key_file != NULL);
 
   g_key_file_clear (key_file);
-  g_key_file_unref (key_file);
+
+  if (g_atomic_int_dec_and_test (&key_file->ref_count))
+    g_slice_free (GKeyFile, key_file);
+  else
+    g_key_file_init (key_file);
 }
 
 /**
diff --git a/glib/tests/keyfile.c b/glib/tests/keyfile.c
index 16f3b788b..9ee7d02c4 100644
--- a/glib/tests/keyfile.c
+++ b/glib/tests/keyfile.c
@@ -1726,6 +1726,42 @@ test_get_locale (void)
   g_key_file_free (kf);
 }
 
+static void
+test_free_when_not_last_ref (void)
+{
+  GKeyFile *kf;
+  GError *error = NULL;
+  const gchar *data =
+    "[Group]\n"
+    "Key=Value\n";
+
+  kf = load_data (data, G_KEY_FILE_NONE);
+  /* Add a second ref */
+  g_key_file_ref (kf);
+
+  /* Quick coherence check */
+  g_assert_true (g_key_file_has_group (kf, "Group"));
+  g_assert_true (g_key_file_has_key (kf, "Group", "Key", &error));
+  g_assert_no_error (error);
+
+  /* Should clear all keys and groups, and remove one ref */
+  g_key_file_free (kf);
+
+  /* kf should still work */
+  g_assert_false (g_key_file_has_group (kf, "Group"));
+  g_assert_false (g_key_file_has_key (kf, "Group", "Key", &error));
+  check_error (&error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_GROUP_NOT_FOUND);
+  g_clear_error (&error);
+
+  g_key_file_load_from_data (kf, data, -1, G_KEY_FILE_NONE, &error);
+  g_assert_no_error (error);
+
+  g_assert_true (g_key_file_has_group (kf, "Group"));
+  g_assert_true (g_key_file_has_key (kf, "Group", "Key", &error));
+
+  g_key_file_unref (kf);
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -1771,6 +1807,7 @@ main (int argc, char *argv[])
   g_test_add_func ("/keyfile/roundtrip", test_roundtrip);
   g_test_add_func ("/keyfile/bytes", test_bytes);
   g_test_add_func ("/keyfile/get-locale", test_get_locale);
+  g_test_add_func ("/keyfile/free-when-not-last-ref", test_free_when_not_last_ref);
 
   return g_test_run ();
 }