125 lines
4.8 KiB
Diff
125 lines
4.8 KiB
Diff
From 2a4f33df723d4b9ce68e5948b568a89675d37411 Mon Sep 17 00:00:00 2001
|
|
From: Christian Hergert <chergert@redhat.com>
|
|
Date: Wed, 26 Feb 2020 14:46:20 -0800
|
|
Subject: [PATCH 4/6] global: force fsync() to worker thread when saving state
|
|
|
|
The g_file_replace_contents_async() API can potentially call fsync() from
|
|
the thread calling into it upon completion. This can have disasterous
|
|
effects when run from the compositor main thread such as complete stalls.
|
|
|
|
This is a followup to 86a00b6872375a266449beee1ea6d5e94f1ebbcb which
|
|
assumed (like the rest of us) that the fsync() would be performed on the
|
|
thread that was doing the I/O operations.
|
|
|
|
You can verify this with an strace -e fsync and cause terminal to display
|
|
a command completed notification (eg: from a backdrop window).
|
|
|
|
This also fixes a lifecycle bug for the variant, as
|
|
g_file_replace_contents_async() does not copy the data during the operation
|
|
as that is the responsibility of the caller. Instead, we just use a GBytes
|
|
variant and reference the variant there.
|
|
|
|
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/1050
|
|
---
|
|
src/shell-global.c | 70 +++++++++++++++++++++++++++++++++++++++++-----
|
|
1 file changed, 63 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/src/shell-global.c b/src/shell-global.c
|
|
index df84b6b0d..4b33778e0 100644
|
|
--- a/src/shell-global.c
|
|
+++ b/src/shell-global.c
|
|
@@ -1572,6 +1572,55 @@ delete_variant_cb (GObject *object,
|
|
g_hash_table_remove (global->save_ops, object);
|
|
}
|
|
|
|
+static void
|
|
+replace_contents_worker (GTask *task,
|
|
+ gpointer source_object,
|
|
+ gpointer task_data,
|
|
+ GCancellable *cancellable)
|
|
+{
|
|
+ GFile *file = source_object;
|
|
+ GBytes *bytes = task_data;
|
|
+ GError *error = NULL;
|
|
+ const gchar *data;
|
|
+ gsize len;
|
|
+
|
|
+ data = g_bytes_get_data (bytes, &len);
|
|
+
|
|
+ if (!g_file_replace_contents (file, data, len, NULL, FALSE,
|
|
+ G_FILE_CREATE_REPLACE_DESTINATION,
|
|
+ NULL, cancellable, &error))
|
|
+ g_task_return_error (task, g_steal_pointer (&error));
|
|
+ else
|
|
+ g_task_return_boolean (task, TRUE);
|
|
+}
|
|
+
|
|
+static void
|
|
+replace_contents_async (GFile *path,
|
|
+ GBytes *bytes,
|
|
+ GCancellable *cancellable,
|
|
+ GAsyncReadyCallback callback,
|
|
+ gpointer user_data)
|
|
+{
|
|
+ g_autoptr(GTask) task = NULL;
|
|
+
|
|
+ g_assert (G_IS_FILE (path));
|
|
+ g_assert (bytes != NULL);
|
|
+ g_assert (!cancellable || G_IS_CANCELLABLE (cancellable));
|
|
+
|
|
+ task = g_task_new (path, cancellable, callback, user_data);
|
|
+ g_task_set_source_tag (task, replace_contents_async);
|
|
+ g_task_set_task_data (task, g_bytes_ref (bytes), (GDestroyNotify)g_bytes_unref);
|
|
+ g_task_run_in_thread (task, replace_contents_worker);
|
|
+}
|
|
+
|
|
+static gboolean
|
|
+replace_contents_finish (GFile *file,
|
|
+ GAsyncResult *result,
|
|
+ GError **error)
|
|
+{
|
|
+ return g_task_propagate_boolean (G_TASK (result), error);
|
|
+}
|
|
+
|
|
static void
|
|
replace_variant_cb (GObject *object,
|
|
GAsyncResult *result,
|
|
@@ -1580,7 +1629,7 @@ replace_variant_cb (GObject *object,
|
|
ShellGlobal *global = user_data;
|
|
GError *error = NULL;
|
|
|
|
- if (!g_file_replace_contents_finish (G_FILE (object), result, NULL, &error))
|
|
+ if (!replace_contents_finish (G_FILE (object), result, &error))
|
|
{
|
|
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
|
|
{
|
|
@@ -1616,12 +1665,19 @@ save_variant (ShellGlobal *global,
|
|
}
|
|
else
|
|
{
|
|
- g_file_replace_contents_async (path,
|
|
- g_variant_get_data (variant),
|
|
- g_variant_get_size (variant),
|
|
- NULL, FALSE,
|
|
- G_FILE_CREATE_REPLACE_DESTINATION,
|
|
- cancellable, replace_variant_cb, global);
|
|
+ g_autoptr(GBytes) bytes = NULL;
|
|
+
|
|
+ bytes = g_bytes_new_with_free_func (g_variant_get_data (variant),
|
|
+ g_variant_get_size (variant),
|
|
+ (GDestroyNotify)g_variant_unref,
|
|
+ g_variant_ref (variant));
|
|
+ /* g_file_replace_contents_async() can potentially fsync() from the
|
|
+ * calling thread when completing the asynchronous task. Instead, we
|
|
+ * want to force that fsync() to a thread to avoid blocking the
|
|
+ * compository main loop. Using our own replace_contents_async()
|
|
+ * simply executes the operation synchronously from a thread.
|
|
+ */
|
|
+ replace_contents_async (path, bytes, cancellable, replace_variant_cb, global);
|
|
}
|
|
|
|
g_object_unref (path);
|
|
--
|
|
2.26.2
|
|
|