gsettings: Fix leaks and assertion on range binding failures

When using g_settings_bind(), if a range binding triggers a range check
failure, g_settings_binding_property_changed() will return early, but it
won't cleanup properly causing some leaks. The binding will also still
be marked as 'running', which causes an assertion failure when trying to
free it:
"g_settings_binding_free: assertion failed: (!binding->running)"

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>

https://bugzilla.gnome.org/show_bug.cgi?id=794805

gio/gsettings.c

@@ -2682,6 +2682,7 @@ g_settings_binding_property_changed (GObject          *object,
   GSettingsBinding *binding = user_data;
   GValue value = G_VALUE_INIT;
   GVariant *variant;
+  gboolean valid = TRUE;
 
   g_assert (object == binding->object);
   g_assert (pspec == binding->property);
@@ -2700,24 +2701,33 @@ g_settings_binding_property_changed (GObject          *object,
 
       if (!g_settings_schema_key_type_check (&binding->key, variant))
         {
+          gchar *type_str;
+          type_str = g_variant_type_dup_string (binding->key.type);
           g_critical ("binding mapping function for key '%s' returned "
                       "GVariant of type '%s' when type '%s' was requested",
                       binding->key.name, g_variant_get_type_string (variant),
-                      g_variant_type_dup_string (binding->key.type));
-          return;
+                      type_str);
+          g_free (type_str);
+          valid = FALSE;
         }
 
-      if (!g_settings_schema_key_range_check (&binding->key, variant))
+      if (valid && !g_settings_schema_key_range_check (&binding->key, variant))
         {
+          gchar *variant_str;
+          variant_str = g_variant_print (variant, TRUE);
           g_critical ("GObject property '%s' on a '%s' object is out of "
                       "schema-specified range for key '%s' of '%s': %s",
                       binding->property->name, g_type_name (binding->property->owner_type),
                       binding->key.name, g_settings_schema_get_id (binding->key.schema),
-                      g_variant_print (variant, TRUE));
-          return;
+                      variant_str);
+          g_free (variant_str);
+          valid = FALSE;
         }
 
-      g_settings_write_to_backend (binding->settings, &binding->key, variant);
+      if (valid)
+        {
+          g_settings_write_to_backend (binding->settings, &binding->key, variant);
+        }
       g_variant_unref (variant);
     }
   g_value_unset (&value);

gio/tests/gsettings.c

@@ -1324,6 +1324,33 @@ test_simple_binding (void)
   g_object_get (obj, "flags", &i, NULL);
   g_assert_cmpint (i, ==, TEST_FLAGS_MOURNING | TEST_FLAGS_WALKING);
 
+  g_settings_bind (settings, "uint", obj, "uint", G_SETTINGS_BIND_DEFAULT);
+
+  g_object_set (obj, "uint", 12345, NULL);
+  g_assert_cmpuint (g_settings_get_uint (settings, "uint"), ==, 12345);
+
+  g_settings_set_uint (settings, "uint", 54321);
+  u = 1111;
+  g_object_get (obj, "uint", &u, NULL);
+  g_assert_cmpuint (u, ==, 54321);
+
+  g_settings_bind (settings, "range", obj, "uint", G_SETTINGS_BIND_DEFAULT);
+  g_object_set (obj, "uint", 22, NULL);
+  u = 1111;
+  g_assert_cmpuint (g_settings_get_uint (settings, "range"), ==, 22);
+  g_object_get (obj, "uint", &u, NULL);
+  g_assert_cmpuint (u, ==, 22);
+
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                         "* is out of schema-specified range for*");
+  g_object_set (obj, "uint", 45, NULL);
+  g_test_assert_expected_messages ();
+  u = 1111;
+  g_object_get (obj, "uint", &u, NULL);
+  g_assert_cmpuint (g_settings_get_uint (settings, "range"), ==, 22);
+  /* The value of the object is currently not reset back to its initial value
+  g_assert_cmpuint (u, ==, 22); */
+
   g_object_unref (obj);
   g_object_unref (settings);
 }
@@ -1472,6 +1499,14 @@ bool_to_string (const GValue       *value,
     return g_variant_new_string ("false");
 }
 
+static GVariant *
+bool_to_bool (const GValue       *value,
+              const GVariantType *expected_type,
+              gpointer            user_data)
+{
+  return g_variant_new_boolean (g_value_get_boolean (value));
+}
+
 /* Test custom bindings.
  * Translate strings to booleans and back
  */
@@ -1508,6 +1543,17 @@ test_custom_binding (void)
   g_assert_cmpstr (s, ==, "true");
   g_free (s);
 
+  g_settings_bind_with_mapping (settings, "string",
+                                obj, "bool",
+                                G_SETTINGS_BIND_DEFAULT,
+                                string_to_bool, bool_to_bool,
+                                NULL, NULL);
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                         "*binding mapping function for key 'string' returned"
+                         " GVariant of type 'b' when type 's' was requested*");
+  g_object_set (obj, "bool", FALSE, NULL);
+  g_test_assert_expected_messages ();
+
   g_object_unref (obj);
   g_object_unref (settings);
 }

gio/tests/org.gtk.test.gschema.xml.orig

@@ -131,6 +131,10 @@
     <key name="flags" flags="org.gtk.test.TestFlags">
       <default>['mourning', 'laughing']</default>
     </key>
+    <key name="range" type='u'>
+      <default>33</default>
+      <range min="2" max="44"/>
+    </key>
   </schema>
 
   <schema id='org.gtk.test.enums' path='/tests/enums/'>