Merge branch '1454-gvdb-corruption' into 'master'

service: Allow opening corrupt GVDB files when writing

See merge request GNOME/dconf!8

19 files changed, 566 insertions, 146 deletions

diff --git a/client/meson.build b/client/meson.build
index ce425d2..74fb090 100644
--- a/client/meson.build
+++ b/client/meson.build
@@ -12,8 +12,6 @@ install_headers(
 
 sources = files('dconf-client.c')
 
-cflags = '-DG_LOG_DOMAIN="dconf"'
-
 deps = [
   libdconf_common_hidden_dep,
   libdconf_gdbus_thread_dep,
@@ -24,7 +22,7 @@ libdconf_client = static_library(
   sources: sources,
   include_directories: top_inc,
   dependencies: libdconf_gdbus_thread_dep,
-  c_args: cflags,
+  c_args: dconf_c_args,
   pic: true,
 )
 
@@ -40,7 +38,7 @@ libdconf = shared_library(
   soversion: soversion,
   include_directories: top_inc,
   dependencies: deps,
-  c_args: cflags,
+  c_args: dconf_c_args,
   install: true,
 )
 
diff --git a/common/meson.build b/common/meson.build
index 90245b7..58e0fa8 100644
--- a/common/meson.build
+++ b/common/meson.build
@@ -17,14 +17,12 @@ sources = files(
   'dconf-paths.c',
 )
 
-cflags = ['-DG_LOG_DOMAIN="dconf"']
-
 libdconf_common = static_library(
   'dconf-common',
   sources: sources,
   include_directories: top_inc,
   dependencies: glib_dep,
-  c_args: cflags,
+  c_args: dconf_c_args,
   pic: true,
 )
 
@@ -33,14 +31,12 @@ libdconf_common_dep = declare_dependency(
   link_whole: libdconf_common,
 )
 
-cflags += cc.get_supported_arguments('-fvisibility=hidden')
-
 libdconf_common_hidden = static_library(
   'dconf-common-hidden',
   sources: sources,
   include_directories: top_inc,
   dependencies: glib_dep,
-  c_args: cflags,
+  c_args: dconf_c_args + cc.get_supported_arguments('-fvisibility=hidden'),
   pic: true,
 )
 
diff --git a/engine/meson.build b/engine/meson.build
index d578f3f..d1a959d 100644
--- a/engine/meson.build
+++ b/engine/meson.build
@@ -18,7 +18,7 @@ libdconf_engine = static_library(
   sources: sources,
   include_directories: top_inc,
   dependencies: engine_deps + [libdconf_shm_dep],
-  c_args: '-DG_LOG_DOMAIN="dconf"',
+  c_args: dconf_c_args,
   pic: true,
 )
 
diff --git a/gdbus/meson.build b/gdbus/meson.build
index ca07019..4fbf3ec 100644
--- a/gdbus/meson.build
+++ b/gdbus/meson.build
@@ -1,11 +1,9 @@
-cflags = '-DG_LOG_DOMAIN="dconf"'
-
 libdconf_gdbus_thread = static_library(
   'dconf-gdbus-thread',
   sources: 'dconf-gdbus-thread.c',
   include_directories: top_inc,
   dependencies: libdconf_engine_dep,
-  c_args: cflags,
+  c_args: dconf_c_args,
   pic: true,
 )
 
@@ -19,7 +17,7 @@ libdconf_gdbus_filter = static_library(
   sources: 'dconf-gdbus-filter.c',
   include_directories: top_inc,
   dependencies: libdconf_engine_dep,
-  c_args: cflags,
+  c_args: dconf_c_args,
   pic: true,
 )
 
diff --git a/gsettings/meson.build b/gsettings/meson.build
index 8bd6908..a28892d 100644
--- a/gsettings/meson.build
+++ b/gsettings/meson.build
@@ -12,7 +12,7 @@ libdconf_settings = shared_library(
   include_directories: top_inc,
   link_with: backend_deps,
   dependencies: gio_dep,
-  c_args: '-DG_LOG_DOMAIN="dconf"',
+  c_args: dconf_c_args,
   install: true,
   install_dir: gio_module_dir,
 )
diff --git a/gvdb/meson.build b/gvdb/meson.build
index 1309a5b..1a1aba8 100644
--- a/gvdb/meson.build
+++ b/gvdb/meson.build
@@ -7,12 +7,17 @@ gvdb_deps = [
   glib_dep,
 ]
 
+cflags = [
+  '-DG_LOG_DOMAIN="gvdb (via dconf)"',
+  '-DG_LOG_USE_STRUCTURED=1',
+]
+
 libgvdb = static_library(
   'gvdb',
   sources: sources,
   include_directories: top_inc,
   dependencies: gvdb_deps,
-  c_args: '-DG_LOG_DOMAIN="gvdb (via dconf)"',
+  c_args: cflags,
   pic: true,
 )
 
diff --git a/meson.build b/meson.build
index ba14507..788cea9 100644
--- a/meson.build
+++ b/meson.build
@@ -35,6 +35,11 @@ endif
 
 add_project_arguments(common_flags, language: 'c')
 
+dconf_c_args = [
+  '-DG_LOG_DOMAIN="dconf"',
+  '-DG_LOG_USE_STRUCTURED=1',
+]
+
 gio_req_version = '>= 2.25.7'
 
 gio_dep = dependency('gio-2.0', version: gio_req_version)
diff --git a/service/dconf-gvdb-utils.c b/service/dconf-gvdb-utils.c
index 099a9f3..93a4719 100644
--- a/service/dconf-gvdb-utils.c
+++ b/service/dconf-gvdb-utils.c
@@ -26,12 +26,15 @@
 #include "../gvdb/gvdb-builder.h"
 #include "../gvdb/gvdb-reader.h"
 
+#include <errno.h>
+#include <glib.h>
+#include <glib/gstdio.h>
 #include <string.h>
 
 DConfChangeset *
-dconf_gvdb_utils_read_file (const gchar  *filename,
-                            gboolean     *file_missing,
-                            GError      **error)
+dconf_gvdb_utils_read_and_back_up_file (const gchar  *filename,
+                                        gboolean     *file_missing,
+                                        GError      **error)
 {
   DConfChangeset *database;
   GError *my_error = NULL;
@@ -57,7 +60,36 @@ dconf_gvdb_utils_read_file (const gchar  *filename,
   /* Otherwise, we should report errors to prevent ourselves from
    * overwriting the database in other situations...
    */
-  if (my_error)
+  if (g_error_matches (my_error, G_FILE_ERROR, G_FILE_ERROR_INVAL))
+    {
+      /* Move the database to a backup file, warn and continue with a new
+       * database. The alternative is erroring out and exiting the daemon,
+       * which leaves the user’s session essentially unusable.
+       *
+       * The code to find an unused backup filename is racy, but this is an
+       * error handling path. Who cares. */
+      g_autofree gchar *backup_filename = NULL;
+      guint i;
+
+      for (i = 0;
+           i < G_MAXUINT &&
+           (backup_filename == NULL || g_file_test (backup_filename, G_FILE_TEST_EXISTS));
+           i++)
+        {
+          g_free (backup_filename);
+          backup_filename = g_strdup_printf ("%s~%u", filename, i);
+        }
+
+      if (g_rename (filename, backup_filename) != 0)
+        g_warning ("Error renaming corrupt database from ‘%s’ to ‘%s’: %s",
+                   filename, backup_filename, g_strerror (errno));
+      else
+        g_warning ("Database ‘%s’ was corrupt: moved it to ‘%s’ and created an empty replacement",
+                   filename, backup_filename);
+
+      g_clear_error (&my_error);
+    }
+  else if (my_error)
     {
       g_propagate_prefixed_error (error, my_error, "Cannot open dconf database: ");
       return NULL;
diff --git a/service/dconf-gvdb-utils.h b/service/dconf-gvdb-utils.h
index 5b3e3a9..7076781 100644
--- a/service/dconf-gvdb-utils.h
+++ b/service/dconf-gvdb-utils.h
@@ -23,7 +23,7 @@
 
 #include "../common/dconf-changeset.h"
 
-DConfChangeset *                dconf_gvdb_utils_read_file              (const gchar     *filename,
+DConfChangeset *                dconf_gvdb_utils_read_and_back_up_file  (const gchar     *filename,
                                                                          gboolean        *file_missing,
                                                                          GError         **error);
 gboolean                        dconf_gvdb_utils_write_file             (const gchar     *filename,
diff --git a/service/dconf-writer.c b/service/dconf-writer.c
index 00d34fe..5fb3467 100644
--- a/service/dconf-writer.c
+++ b/service/dconf-writer.c
@@ -107,7 +107,7 @@ dconf_writer_real_begin (DConfWriter  *writer,
     {
       gboolean missing;
 
-      writer->priv->commited_values = dconf_gvdb_utils_read_file (writer->priv->filename, &missing, error);
+      writer->priv->commited_values = dconf_gvdb_utils_read_and_back_up_file (writer->priv->filename, &missing, error);
 
       if (!writer->priv->commited_values)
         return FALSE;
diff --git a/service/dconf-writer.h b/service/dconf-writer.h
index a41f115..17360c9 100644
--- a/service/dconf-writer.h
+++ b/service/dconf-writer.h
@@ -20,7 +20,9 @@
 #ifndef __dconf_writer_h__
 #define __dconf_writer_h__
 
+#include <glib.h>
 #include <gio/gio.h>
+#include <gobject/gobject.h>
 
 #include "../common/dconf-changeset.h"
 #include "dconf-generated.h"
@@ -65,6 +67,7 @@ struct _DConfWriter
   DConfWriterPrivate *priv;
 };
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (DConfWriter, g_object_unref)
 
 GType                   dconf_writer_get_type                           (void);
 
diff --git a/service/meson.build b/service/meson.build
index 618cbd5..35ee23a 100644
--- a/service/meson.build
+++ b/service/meson.build
@@ -11,35 +11,45 @@ configure_file(
   install_dir: dbus_session_service_dir,
 )
 
-sources = [
+lib_sources = [
   'dconf-blame.c',
   'dconf-gvdb-utils.c',
   'dconf-keyfile-writer.c',
   'dconf-service.c',
   'dconf-shm-writer.c',
   'dconf-writer.c',
+]
+sources = [
   'main.c',
 ]
 
-sources += gnome.gdbus_codegen(
+lib_sources += gnome.gdbus_codegen(
   'dconf-generated',
   dconf_namespace + '.xml',
   interface_prefix: dconf_namespace + '.',
   namespace: 'DConfDBus',
 )
 
-service_deps = [
-  gio_unix_dep,
-  libdconf_common_dep,
-  libdconf_shm_dep,
-  libgvdb_dep,
-]
+libdconf_service = static_library(
+  'dconf-service',
+  sources: lib_sources,
+  include_directories: top_inc,
+  c_args: dconf_c_args,
+  dependencies: gio_unix_dep,
+  link_with: [
+    libdconf_common,
+    libdconf_shm,
+    libgvdb,
+  ],
+)
 
 executable(
   'dconf-service',
   sources,
   include_directories: top_inc,
-  dependencies: service_deps,
+  c_args: dconf_c_args,
+  dependencies: gio_unix_dep,
+  link_with: libdconf_service,
   install: true,
   install_dir: dconf_libexecdir,
 )
diff --git a/shm/meson.build b/shm/meson.build
index 57a9852..5fb9fe2 100644
--- a/shm/meson.build
+++ b/shm/meson.build
@@ -3,7 +3,7 @@ libdconf_shm = static_library(
   sources: 'dconf-shm.c',
   include_directories: top_inc,
   dependencies: glib_dep,
-  c_args: '-DG_LOG_DOMAIN="dconf"',
+  c_args: dconf_c_args,
   pic: true,
 )
 
diff --git a/tests/client.c b/tests/client.c
index 6390438..4727e0c 100644
--- a/tests/client.c
+++ b/tests/client.c
@@ -86,28 +86,22 @@ fail_one_call (void)
   g_error_free (error);
 }
 
-static void
-log_handler (const gchar    *log_domain,
-             GLogLevelFlags  log_level,
-             const gchar    *message,
-             gpointer        user_data)
+static GLogWriterOutput
+log_writer_cb (GLogLevelFlags   log_level,
+               const GLogField *fields,
+               gsize            n_fields,
+               gpointer         user_data)
 {
-  if (strstr (message, "--expected error from testcase--"))
-    return;
-
-  g_log_default_handler (log_domain, log_level, message, user_data);
-}
+  gsize i;
 
-static gboolean
-fatal_log_handler (const gchar    *log_domain,
-                   GLogLevelFlags  log_level,
-                   const gchar    *message,
-                   gpointer        user_data)
-{
-  if (strstr (message, "--expected error from testcase--"))
-    return FALSE;
+  for (i = 0; i < n_fields; i++)
+    {
+      if (g_strcmp0 (fields[i].key, "MESSAGE") == 0 &&
+          strstr (fields[i].value, "--expected error from testcase--"))
+        return G_LOG_WRITER_HANDLED;
+    }
 
-  return TRUE;
+  return G_LOG_WRITER_UNHANDLED;
 }
 
 static void
@@ -116,8 +110,7 @@ test_fast (void)
   DConfClient *client;
   gint i;
 
-  g_log_set_default_handler (log_handler, NULL);
-  g_test_log_set_fatal_handler (fatal_log_handler, NULL);
+  g_log_set_writer_func (log_writer_cb, NULL, NULL);
 
   client = dconf_client_new ();
   g_signal_connect (client, "changed", G_CALLBACK (changed), NULL);
diff --git a/tests/dbus.c b/tests/dbus.c
index 980d2b0..032cb04 100644
--- a/tests/dbus.c
+++ b/tests/dbus.c
@@ -1,5 +1,3 @@
-#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_36 /* Suppress deprecation warnings */
-
 #include <string.h>
 #include <glib.h>
 #include <stdlib.h>
@@ -147,7 +145,7 @@ dconf_engine_handle_dbus_signal (GBusType     bus_type,
 }
 
 static void
-test_creation_error (void)
+test_creation_error_sync_with_error (void)
 {
   if (g_getenv ("DISPLAY") == NULL || g_strcmp0 (g_getenv ("DISPLAY"), "") == 0)
     {
@@ -156,7 +154,7 @@ test_creation_error (void)
     }
 
   /* Sync with 'error' */
-  if (g_test_trap_fork (0, 0))
+  if (g_test_subprocess ())
     {
       GError *error = NULL;
       GVariant *reply;
@@ -170,13 +168,24 @@ test_creation_error (void)
       g_assert (reply == NULL);
       g_assert (error != NULL);
       g_assert (strstr (error->message, "some nonsense"));
-      exit (0);
+      return;
     }
 
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
+}
+
+static void
+test_creation_error_sync_without_error (void)
+{
+  if (g_getenv ("DISPLAY") == NULL || g_strcmp0 (g_getenv ("DISPLAY"), "") == 0)
+    {
+      g_test_skip ("FIXME: D-Bus tests do not work on CI at the moment");
+      return;
+    }
 
   /* Sync without 'error' */
-  if (g_test_trap_fork (0, 0))
+  if (g_test_subprocess ())
     {
       GVariant *reply;
 
@@ -187,13 +196,24 @@ test_creation_error (void)
                                                 g_variant_new ("()"), G_VARIANT_TYPE ("(as)"), NULL);
 
       g_assert (reply == NULL);
-      exit (0);
+      return;
     }
 
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
+}
+
+static void
+test_creation_error_async (void)
+{
+  if (g_getenv ("DISPLAY") == NULL || g_strcmp0 (g_getenv ("DISPLAY"), "") == 0)
+    {
+      g_test_skip ("FIXME: D-Bus tests do not work on CI at the moment");
+      return;
+    }
 
   /* Async */
-  if (g_test_trap_fork (0, 0))
+  if (g_test_subprocess ())
     {
       DConfEngineCallHandle *handle;
       GError *error = NULL;
@@ -222,9 +242,10 @@ test_creation_error (void)
       else
         g_assert (error != NULL);
 
-      exit (0);
+      return;
     }
 
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
 }
 
@@ -506,7 +527,11 @@ main (int argc, char **argv)
 
   /* test_creation_error absolutely must come first */
   if (!g_str_equal (DBUS_BACKEND, "/libdbus-1"))
-    g_test_add_func (DBUS_BACKEND "/creation/error", test_creation_error);
+    {
+      g_test_add_func (DBUS_BACKEND "/creation/error/sync-with-error", test_creation_error_sync_with_error);
+      g_test_add_func (DBUS_BACKEND "/creation/error/sync-without-error", test_creation_error_sync_without_error);
+      g_test_add_func (DBUS_BACKEND "/creation/error/async", test_creation_error_async);
+    }
 
   g_test_add_func (DBUS_BACKEND "/sync-call/success", test_sync_call_success);
   g_test_add_func (DBUS_BACKEND "/sync-call/error", test_sync_call_error);
diff --git a/tests/engine.c b/tests/engine.c
index b351126..7f2a748 100644
--- a/tests/engine.c
+++ b/tests/engine.c
@@ -1,7 +1,5 @@
 #define _GNU_SOURCE
 
-#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_36 /* Suppress deprecation warnings */
-
 #include "../engine/dconf-engine.h"
 #include "../engine/dconf-engine-profile.h"
 #include "../common/dconf-enums.h"
@@ -37,6 +35,14 @@ fopen (const char *filename,
   return (* real_fopen) (filename, mode);
 }
 
+static void assert_no_messages (void);
+static void assert_pop_message (const gchar    *expected_domain,
+                                GLogLevelFlags  expected_log_level,
+                                const gchar    *expected_message_fragment);
+static void assert_maybe_pop_message (const gchar    *expected_domain,
+                                      GLogLevelFlags  expected_log_level,
+                                      const gchar    *expected_message_fragment);
+
 static GThread *main_thread;
 static GString *change_log;
 
@@ -144,47 +150,39 @@ test_five_times (const gchar *filename,
   g_free (expected_names);
 }
 
+typedef struct
+{
+  const gchar *profile_path;
+  const gchar *expected_stderr_pattern;
+} ProfileParserOpenData;
+
 static void
-test_profile_parser (void)
+test_profile_parser_errors (gconstpointer test_data)
 {
+  const ProfileParserOpenData *data = test_data;
   DConfEngineSource **sources;
   gint n_sources;
 
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
 
-      sources = dconf_engine_profile_open (SRCDIR "/profile/this-file-does-not-exist", &n_sources);
+      sources = dconf_engine_profile_open (data->profile_path, &n_sources);
       g_assert_cmpint (n_sources, ==, 0);
       g_assert (sources == NULL);
-      exit (0);
+      return;
     }
-  g_test_trap_assert_passed ();
-  g_test_trap_assert_stderr ("*WARNING*: unable to open named profile*");
 
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
-    {
-      g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
-
-      sources = dconf_engine_profile_open (SRCDIR "/profile/broken-profile", &n_sources);
-      g_assert_cmpint (n_sources, ==, 0);
-      g_assert (sources == NULL);
-      exit (0);
-    }
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
-  g_test_trap_assert_stderr ("*WARNING*: unknown dconf database*unknown dconf database*");
-
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
-    {
-      g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
+  g_test_trap_assert_stderr (data->expected_stderr_pattern);
+}
 
-      sources = dconf_engine_profile_open (SRCDIR "/profile/gdm", &n_sources);
-      g_assert_cmpint (n_sources, ==, 0);
-      g_assert (sources == NULL);
-      exit (0);
-    }
-  g_test_trap_assert_passed ();
-  g_test_trap_assert_stderr ("*WARNING*: unknown dconf database*unknown dconf database*");
+static void
+test_profile_parser (void)
+{
+  DConfEngineSource **sources;
+  gint n_sources;
 
   test_five_times (SRCDIR "/profile/empty-profile", 0);
   test_five_times (SRCDIR "/profile/test-profile", 1, "test");
@@ -370,12 +368,13 @@ test_file_source (void)
   g_assert (source != NULL);
   g_assert (source->values == NULL);
   g_assert (source->locks == NULL);
-  g_test_expect_message ("dconf", G_LOG_LEVEL_WARNING, "*unable to open file '/path/to/db'*");
   reopened = dconf_engine_source_refresh (source);
   g_assert (source->values == NULL);
   g_assert (source->locks == NULL);
   dconf_engine_source_free (source);
 
+  assert_pop_message ("dconf", G_LOG_LEVEL_WARNING, "unable to open file '/path/to/db'");
+
   source = dconf_engine_source_new ("file-db:/path/to/db");
   g_assert (source != NULL);
   g_assert (source->values == NULL);
@@ -459,7 +458,7 @@ test_service_source (void)
   gboolean reopened;
 
   /* Make sure we deal with errors from the service sensibly */
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
 
@@ -470,8 +469,10 @@ test_service_source (void)
       g_assert (source->locks == NULL);
       reopened = dconf_engine_source_refresh (source);
 
-      exit (0);
+      return;
     }
+
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
   g_test_trap_assert_stderr ("*WARNING*: unable to open file*unknown/nil*expect degraded performance*");
 
@@ -546,7 +547,7 @@ test_system_source (void)
   g_assert (source != NULL);
 
   /* Check to see that we get the warning about the missing file. */
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
 
@@ -571,8 +572,10 @@ test_system_source (void)
 
       dconf_engine_source_free (source);
 
-      exit (0);
+      return;
     }
+
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
   /* Check that we only saw the warning, but only one time. */
   g_test_trap_assert_stderr ("*this gvdb does not exist; expect degraded performance*");
@@ -926,35 +929,6 @@ check_read (DConfEngine *engine,
     }
 }
 
-static gboolean
-is_expected (const gchar    *log_domain,
-             GLogLevelFlags  log_level,
-             const gchar    *message)
-{
-  return g_str_equal (log_domain, "dconf") &&
-         log_level == (G_LOG_LEVEL_WARNING | G_LOG_FLAG_FATAL) &&
-         strstr (message, "unable to open file '" SYSCONFDIR "/dconf/db");
-}
-
-static gboolean
-fatal_handler (const gchar    *log_domain,
-               GLogLevelFlags  log_level,
-               const gchar    *message,
-               gpointer        user_data)
-{
-  return !is_expected (log_domain, log_level, message);
-}
-
-static void
-normal_handler (const gchar    *log_domain,
-                GLogLevelFlags  log_level,
-                const gchar    *message,
-                gpointer        user_data)
-{
-  if (!is_expected (log_domain, log_level, message))
-    g_error ("unexpected error: %s\n", message);
-}
-
 static void
 test_read (void)
 {
@@ -965,13 +939,10 @@ test_read (void)
   DConfEngine *engine;
   guint i, j, k;
   guint n;
-  guint handler_id;
 
   /* This test throws a lot of messages about missing databases.
    * Capture and ignore them.
    */
-  g_test_log_set_fatal_handler (fatal_handler, NULL);
-  handler_id = g_log_set_handler ("dconf", G_LOG_LEVEL_WARNING | G_LOG_FLAG_FATAL, normal_handler, NULL);
 
   /* Our test strategy is as follows:
    *
@@ -1136,6 +1107,9 @@ test_read (void)
               /* Clean up */
               setup_state (n, i, 0, NULL);
               dconf_engine_unref (engine);
+
+              assert_maybe_pop_message ("dconf", G_LOG_LEVEL_WARNING,
+                                        "unable to open file '" SYSCONFDIR "/dconf/db");
             }
       }
 
@@ -1144,7 +1118,7 @@ test_read (void)
   g_free (profile_filename);
   dconf_mock_shm_reset ();
 
-  g_log_remove_handler ("dconf", handler_id);
+  assert_no_messages ();
 }
 
 static void
@@ -1560,13 +1534,14 @@ test_change_fast (void)
   g_variant_unref (value);
 
   /* Fail the attempted write.  This should cause a warning and a change. */
-  g_test_expect_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: something failed");
   error = g_error_new_literal (G_FILE_ERROR, G_FILE_ERROR_NOENT, "something failed");
   dconf_mock_dbus_async_reply (NULL, error);
   g_clear_error (&error);
   g_assert_cmpstr (change_log->str, ==, "/value:1::nil;");
   g_string_set_size (change_log, 0);
 
+  assert_pop_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: something failed");
+
   /* Verify that the value became unset due to the failure */
   value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value");
   g_assert (value == NULL);
@@ -1611,12 +1586,12 @@ test_change_fast (void)
   value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value");
   g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value");
   g_variant_unref (value);
-  g_test_expect_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: something failed");
   error = g_error_new_literal (G_FILE_ERROR, G_FILE_ERROR_NOENT, "something failed");
   dconf_mock_dbus_async_reply (NULL, error);
   g_clear_error (&error);
   g_assert_cmpstr (change_log->str, ==, "/:2:to-reset,value:nil;");
   g_string_set_size (change_log, 0);
+  assert_pop_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: something failed");
   value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value");
   g_assert (value == NULL);
   dconf_mock_dbus_assert_no_async ();
@@ -1891,12 +1866,13 @@ test_sync (void)
   /* The write will try to check the system-db for a lock.  That will
    * fail because it doesn't exist...
    */
-  g_test_expect_message ("dconf", G_LOG_LEVEL_WARNING, "*unable to open file*");
   change = dconf_changeset_new_write ("/value", g_variant_new_boolean (TRUE));
   success = dconf_engine_change_fast (engine, change, NULL, &error);
   g_assert_no_error (error);
   g_assert (success);
 
+  assert_pop_message ("dconf", G_LOG_LEVEL_WARNING, "unable to open file");
+
   /* Spin up some waiters */
   for (i = 0; i < G_N_ELEMENTS (waiter_threads); i++)
     waiter_threads[i] = g_thread_new ("test waiter", waiter_thread, engine);
@@ -1917,10 +1893,12 @@ test_sync (void)
     waiter_threads[i] = g_thread_new ("test waiter", waiter_thread, engine);
   g_usleep(100 * G_TIME_SPAN_MILLISECOND);
   error = g_error_new_literal (G_FILE_ERROR, G_FILE_ERROR_NOENT, "some error");
-  g_test_expect_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: some error");
   g_atomic_int_set (&it_is_good_to_be_done, TRUE);
   dconf_mock_dbus_async_reply (NULL, error);
   g_clear_error (&error);
+
+  assert_pop_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: some error");
+
   /* Make sure they all quit by joining them */
   for (i = 0; i < G_N_ELEMENTS (waiter_threads); i++)
     g_thread_join (waiter_threads[i]);
@@ -1956,18 +1934,149 @@ test_sync (void)
   dconf_mock_shm_reset ();
 }
 
+/* Log handling. */
+typedef struct
+{
+  GLogLevelFlags log_level;
+  GLogField *fields;
+  gsize n_fields;
+} LogMessage;
+
+static void
+log_message_clear (LogMessage *message)
+{
+  gsize i;
+
+  for (i = 0; i < message->n_fields; i++)
+    {
+      g_free ((gpointer) message->fields[i].key);
+      g_free ((gpointer) message->fields[i].value);
+    }
+}
+
+static GArray *logged_messages = NULL;
+
+static GLogWriterOutput
+log_writer_cb (GLogLevelFlags   log_level,
+               const GLogField *fields,
+               gsize            n_fields,
+               gpointer         user_data)
+{
+  LogMessage *message;
+  gsize i;
+
+  /* If we’re running as a subprocess, the parent process is going to be
+   * checking our stderr, so just behave normally. */
+  if (g_test_subprocess ())
+    return g_log_writer_default (log_level, fields, n_fields, user_data);
+
+  /* We only care about dconf messages and non-debug messages. */
+  if (log_level == G_LOG_LEVEL_DEBUG)
+    return G_LOG_WRITER_HANDLED;
+
+  for (i = 0; i < n_fields; i++)
+    {
+      if (g_strcmp0 (fields[i].key, "GLIB_DOMAIN") == 0 &&
+          g_strcmp0 (fields[i].value, "dconf") != 0)
+        return G_LOG_WRITER_HANDLED;
+    }
+
+  /* Append the message to the queue. */
+  g_array_set_size (logged_messages, logged_messages->len + 1);
+  message = &g_array_index (logged_messages, LogMessage, logged_messages->len - 1);
+
+  message->log_level = log_level;
+  message->fields = g_new0 (GLogField, n_fields);
+  message->n_fields = n_fields;
+
+  for (i = 0; i < n_fields; i++)
+    {
+      gsize length = (fields[i].length < 0) ? strlen (fields[i].value) + 1 : fields[i].length;
+      message->fields[i].key = g_strdup (fields[i].key);
+      message->fields[i].value = g_malloc0 (length);
+      memcpy ((gpointer) message->fields[i].value, fields[i].value, length);
+      message->fields[i].length = fields[i].length;
+    }
+
+  return G_LOG_WRITER_HANDLED;
+}
+
+/* Assert there are no logged messages in the queue. */
+static void
+assert_no_messages (void)
+{
+  g_assert_cmpuint (logged_messages->len, ==, 0);
+}
+
+/* Assert there is at least one logged message in the queue, and the oldest
+ * logged message matches the given expectations. If so, pop it from the queue;
+ * if not, abort. */
+static void
+assert_pop_message (const gchar    *expected_domain,
+                    GLogLevelFlags  expected_log_level,
+                    const gchar    *expected_message_fragment)
+{
+  const LogMessage *logged_message;
+  gsize i;
+  const gchar *message = NULL, *domain = NULL;
+
+  g_assert_cmpuint (logged_messages->len, >, 0);
+  logged_message = &g_array_index (logged_messages, LogMessage, 0);
+
+  for (i = 0; i < logged_message->n_fields; i++)
+    {
+      if (g_strcmp0 (logged_message->fields[i].key, "MESSAGE") == 0)
+        message = logged_message->fields[i].value;
+      else if (g_strcmp0 (logged_message->fields[i].key, "GLIB_DOMAIN") == 0)
+        domain = logged_message->fields[i].value;
+    }
+
+  g_assert_cmpstr (domain, ==, expected_domain);
+  g_assert_cmpuint (logged_message->log_level, ==, expected_log_level);
+  g_assert_cmpstr (strstr (message, expected_message_fragment), !=, NULL);
+
+  g_array_remove_index (logged_messages, 0);
+}
+
+/* If there is at least one logged message in the queue, act like
+ * assert_pop_message(). Otherwise, if the queue is empty, return. */
+static void
+assert_maybe_pop_message (const gchar    *expected_domain,
+                          GLogLevelFlags  expected_log_level,
+                          const gchar    *expected_message_fragment)
+{
+  if (logged_messages->len == 0)
+    return;
+
+  assert_pop_message (expected_domain, expected_log_level, expected_message_fragment);
+}
 
 int
 main (int argc, char **argv)
 {
+  const ProfileParserOpenData profile_parser0 =
+    { SRCDIR "/profile/this-file-does-not-exist", "*WARNING*: unable to open named profile*" };
+  const ProfileParserOpenData profile_parser1 =
+    { SRCDIR "/profile/broken-profile", "*WARNING*: unknown dconf database*unknown dconf database*" };
+  const ProfileParserOpenData profile_parser2 =
+    { SRCDIR "/profile/gdm", "*WARNING*: unknown dconf database*unknown dconf database*" };
+  int retval;
+
   g_setenv ("XDG_RUNTIME_DIR", "/RUNTIME/", TRUE);
   g_setenv ("XDG_CONFIG_HOME", "/HOME/.config", TRUE);
   g_unsetenv ("DCONF_PROFILE");
 
+  logged_messages = g_array_new (FALSE, FALSE, sizeof (LogMessage));
+  g_array_set_clear_func (logged_messages, (GDestroyNotify) log_message_clear);
+  g_log_set_writer_func (log_writer_cb, NULL, NULL);
+
   main_thread = g_thread_self ();
 
   g_test_init (&argc, &argv, NULL);
 
+  g_test_add_data_func ("/engine/profile-parser/errors/0", &profile_parser0, test_profile_parser_errors);
+  g_test_add_data_func ("/engine/profile-parser/errors/1", &profile_parser1, test_profile_parser_errors);
+  g_test_add_data_func ("/engine/profile-parser/errors/2", &profile_parser2, test_profile_parser_errors);
   g_test_add_func ("/engine/profile-parser", test_profile_parser);
   g_test_add_func ("/engine/signal-threadsafety", test_signal_threadsafety);
   g_test_add_func ("/engine/sources/user", test_user_source);
@@ -1985,5 +2094,10 @@ main (int argc, char **argv)
   g_test_add_func ("/engine/signals", test_signals);
   g_test_add_func ("/engine/sync", test_sync);
 
-  return g_test_run ();
+  retval = g_test_run ();
+
+  assert_no_messages ();
+  g_array_unref (logged_messages);
+
+  return retval;
 }
diff --git a/tests/meson.build b/tests/meson.build
index ef0b940..3274059 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -30,6 +30,7 @@ unit_tests = [
   ['gdbus-filter', 'dbus.c', '-DDBUS_BACKEND="/gdbus/filter"', libdconf_gdbus_filter_dep, []],
   ['engine', 'engine.c', '-DSRCDIR="@0@"'.format(test_dir), [dl_dep, libdconf_engine_dep, m_dep], libdconf_mock],
   ['client', 'client.c', '-DSRCDIR="@0@"'.format(test_dir), [libdconf_client_dep, libdconf_engine_dep], libdconf_mock],
+  ['writer', 'writer.c', '-DSRCDIR="@0@"'.format(test_dir), [glib_dep, dl_dep, m_dep], [libdconf_service, libdconf_mock]],
 ]
 
 foreach unit_test: unit_tests
@@ -39,6 +40,7 @@ foreach unit_test: unit_tests
     c_args: unit_test[2],
     dependencies: unit_test[3],
     link_with: unit_test[4],
+    include_directories: [top_inc, include_directories('../service')],
   )
 
   test(unit_test[0], exe, is_parallel: false, env: envs)
diff --git a/tests/shm.c b/tests/shm.c
index 66e67a2..a0cf67e 100644
--- a/tests/shm.c
+++ b/tests/shm.c
@@ -1,7 +1,5 @@
 #define _GNU_SOURCE
 
-#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_36 /* Suppress deprecation warnings */
-
 #include "../common/dconf-paths.h"
 #include <glib/gstdio.h>
 #include <sys/stat.h>
@@ -19,7 +17,7 @@ test_mkdir_fail (void)
 {
   guint8 *shm;
 
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       gchar *evil;
       gint fd;
@@ -36,8 +34,10 @@ test_mkdir_fail (void)
       g_unlink (evil);
       g_free (evil);
 
-      exit (0);
+      return;
     }
+
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
   g_test_trap_assert_stderr ("*unable to create directory*");
 }
@@ -64,7 +64,7 @@ test_open_and_flag (void)
 static void
 test_invalid_name (void)
 {
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       guint8 *shm;
 
@@ -73,8 +73,10 @@ test_invalid_name (void)
       shm = dconf_shm_open ("foo/bar");
       g_assert (shm == NULL);
       g_assert (dconf_shm_is_flagged (shm));
-      exit (0);
+      return;
     }
+
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
   g_test_trap_assert_stderr ("*unable to create*foo/bar*");
 }
@@ -107,7 +109,7 @@ pwrite (int fd, const void *buf, size_t count, off_t offset)
 static void
 test_out_of_space_open (void)
 {
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       guint8 *shm;
 
@@ -117,8 +119,10 @@ test_out_of_space_open (void)
       shm = dconf_shm_open ("foo");
       g_assert (shm == NULL);
       g_assert (dconf_shm_is_flagged (shm));
-      exit (0);
+      return;
     }
+
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
   g_test_trap_assert_stderr ("*failed to allocate*foo*");
 }
@@ -126,14 +130,16 @@ test_out_of_space_open (void)
 static void
 test_out_of_space_flag (void)
 {
-  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+  if (g_test_subprocess ())
     {
       g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
       should_fail_pwrite = TRUE;
 
       dconf_shm_flag ("foo");
-      exit (0);
+      return;
     }
+
+  g_test_trap_subprocess (NULL, 0, 0);
   g_test_trap_assert_passed ();
 }
 
diff --git a/tests/writer.c b/tests/writer.c
new file mode 100644
index 0000000..955ba91
--- /dev/null
+++ b/tests/writer.c
@@ -0,0 +1,233 @@
+/*
+ * Copyright © 2018 Endless Mobile, Inc
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the licence, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Philip Withnall <withnall@endlessm.com>
+ */
+
+#include <glib.h>
+#include <glib/gstdio.h>
+#include <locale.h>
+
+#include "service/dconf-generated.h"
+#include "service/dconf-writer.h"
+
+static guint n_warnings = 0;
+
+static GLogWriterOutput
+log_writer_cb (GLogLevelFlags   log_level,
+               const GLogField *fields,
+               gsize            n_fields,
+               gpointer         user_data)
+{
+  if (log_level & G_LOG_LEVEL_WARNING)
+    n_warnings++;
+
+  return G_LOG_WRITER_HANDLED;
+}
+
+static void
+assert_n_warnings (guint expected_n_warnings)
+{
+  g_assert_cmpuint (n_warnings, ==, expected_n_warnings);
+  n_warnings = 0;
+}
+
+typedef struct
+{
+  gchar *dconf_dir;  /* (owned) */
+} Fixture;
+
+gchar *config_dir = NULL;
+
+static void
+set_up (Fixture       *fixture,
+        gconstpointer  test_data)
+{
+  fixture->dconf_dir = g_build_filename (config_dir, "dconf", NULL);
+  g_assert_cmpint (g_mkdir (fixture->dconf_dir, 0755), ==, 0);
+
+  g_test_message ("Using dconf directory: %s", fixture->dconf_dir);
+}
+
+static void
+tear_down (Fixture       *fixture,
+           gconstpointer  test_data)
+{
+  g_assert_cmpint (g_rmdir (fixture->dconf_dir), ==, 0);
+  g_clear_pointer (&fixture->dconf_dir, g_free);
+
+  assert_n_warnings (0);
+}
+
+/* Test basic initialisation of a #DConfWriter. This is essentially a smoketest. */
+static void
+test_writer_basic (Fixture       *fixture,
+                   gconstpointer  test_data)
+{
+  g_autoptr(DConfWriter) writer = NULL;
+
+  writer = DCONF_WRITER (dconf_writer_new (DCONF_TYPE_WRITER, "some-name"));
+  g_assert_nonnull (writer);
+
+  g_assert_cmpstr (dconf_writer_get_name (writer), ==, "some-name");
+}
+
+/* Test that beginning a write operation when no database exists succeeds. Note
+ * that the database will not actually be created until some changes are made
+ * and the write is committed. */
+static void
+test_writer_begin_missing (Fixture       *fixture,
+                           gconstpointer  test_data)
+{
+  g_autoptr(DConfWriter) writer = NULL;
+  DConfWriterClass *writer_class;
+  gboolean retval;
+  g_autoptr(GError) local_error = NULL;
+  g_autofree gchar *db_filename = g_build_filename (fixture->dconf_dir, "missing", NULL);
+
+  /* Check the database doesn’t exist. */
+  g_assert_false (g_file_test (db_filename, G_FILE_TEST_EXISTS));
+
+  /* Create a writer. */
+  writer = DCONF_WRITER (dconf_writer_new (DCONF_TYPE_WRITER, "missing"));
+  g_assert_nonnull (writer);
+
+  writer_class = DCONF_WRITER_GET_CLASS (writer);
+  retval = writer_class->begin (writer, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_true (retval);
+}
+
+/* Test that beginning a write operation when a corrupt or empty database exists
+ * will take a backup of the database and then succeed. Note that a new empty
+ * database will not actually be created until some changes are made and the
+ * write is committed. */
+typedef struct
+{
+  const gchar *corrupt_db_contents;
+  guint n_existing_backups;
+} BeginCorruptFileData;
+
+static void
+test_writer_begin_corrupt_file (Fixture       *fixture,
+                                gconstpointer  test_data)
+{
+  const BeginCorruptFileData *data = test_data;
+  g_autoptr(DConfWriter) writer = NULL;
+  DConfWriterClass *writer_class;
+  gboolean retval;
+  g_autoptr(GError) local_error = NULL;
+  g_autofree gchar *db_filename = g_build_filename (fixture->dconf_dir, "corrupt", NULL);
+  g_autofree gchar *new_db_filename_backup = NULL;
+  g_autofree gchar *backup_file_contents = NULL;
+  gsize backup_file_contents_len = 0;
+  guint i;
+
+  /* Create a corrupt database. */
+  g_file_set_contents (db_filename, data->corrupt_db_contents, -1, &local_error);
+  g_assert_no_error (local_error);
+
+  /* Create any existing backups, to test we don’t overwrite them. */
+  for (i = 0; i < data->n_existing_backups; i++)
+    {
+      g_autofree gchar *db_filename_backup = g_strdup_printf ("%s~%u", db_filename, i);
+      g_file_set_contents (db_filename_backup, "backup", -1, &local_error);
+      g_assert_no_error (local_error);
+    }
+
+  new_db_filename_backup = g_strdup_printf ("%s~%u", db_filename, data->n_existing_backups);
+
+  /* Create a writer. */
+  writer = DCONF_WRITER (dconf_writer_new (DCONF_TYPE_WRITER, "corrupt"));
+  g_assert_nonnull (writer);
+
+  writer_class = DCONF_WRITER_GET_CLASS (writer);
+  retval = writer_class->begin (writer, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_true (retval);
+
+  /* The writer should have printed a warning about the corrupt database. */
+  assert_n_warnings (1);
+
+  /* Check a backup file has been created and has the right content. */
+  g_file_get_contents (new_db_filename_backup, &backup_file_contents,
+                       &backup_file_contents_len, &local_error);
+  g_assert_no_error (local_error);
+  g_assert_cmpstr (backup_file_contents, ==, data->corrupt_db_contents);
+  g_assert_cmpuint (backup_file_contents_len, ==, strlen (data->corrupt_db_contents));
+
+  /* Clean up. */
+  g_assert_cmpint (g_unlink (new_db_filename_backup), ==, 0);
+
+  for (i = 0; i < data->n_existing_backups; i++)
+    {
+      g_autofree gchar *db_filename_backup = g_strdup_printf ("%s~%u", db_filename, i);
+      g_assert_cmpint (g_unlink (db_filename_backup), ==, 0);
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  g_autoptr(GError) local_error = NULL;
+  int retval;
+  const BeginCorruptFileData empty_data = { "", 0 };
+  const BeginCorruptFileData corrupt_file_data0 = {
+    "secretly not a valid GVDB database 😧", 0
+  };
+  const BeginCorruptFileData corrupt_file_data1 = {
+    "secretly not a valid GVDB database 😧", 1
+  };
+  const BeginCorruptFileData corrupt_file_data2 = {
+    "secretly not a valid GVDB database 😧", 2
+  };
+
+  setlocale (LC_ALL, "");
+
+  g_test_init (&argc, &argv, NULL);
+
+  /* Set up a fake $XDG_CONFIG_HOME. We can’t do this in the fixture, as
+   * g_get_user_config_dir() caches its return value. */
+  config_dir = g_dir_make_tmp ("dconf-test-writer_XXXXXX", &local_error);
+  g_assert_no_error (local_error);
+  g_assert_true (g_setenv ("XDG_CONFIG_HOME", config_dir, TRUE));
+  g_test_message ("Using config directory: %s", config_dir);
+
+  /* Log handling so we don’t abort on the first g_warning(). */
+  g_log_set_writer_func (log_writer_cb, NULL, NULL);
+
+  g_test_add ("/writer/basic", Fixture, NULL, set_up,
+              test_writer_basic, tear_down);
+  g_test_add ("/writer/begin/missing", Fixture, NULL, set_up,
+              test_writer_begin_missing, tear_down);
+  g_test_add ("/writer/begin/empty", Fixture, &empty_data, set_up,
+              test_writer_begin_corrupt_file, tear_down);
+  g_test_add ("/writer/begin/corrupt-file/0", Fixture, &corrupt_file_data0, set_up,
+              test_writer_begin_corrupt_file, tear_down);
+  g_test_add ("/writer/begin/corrupt-file/1", Fixture, &corrupt_file_data1, set_up,
+              test_writer_begin_corrupt_file, tear_down);
+  g_test_add ("/writer/begin/corrupt-file/2", Fixture, &corrupt_file_data2, set_up,
+              test_writer_begin_corrupt_file, tear_down);
+
+  retval = g_test_run ();
+
+  /* Clean up the config dir. */
+  g_unsetenv ("XDG_CONFIG_HOME");
+  g_assert_cmpint (g_rmdir (config_dir), ==, 0);
+  g_clear_pointer (&config_dir, g_free);
+
+  return retval;
+}