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 86a00b6872
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
This commit is contained in:
parent
24f400d28a
commit
f3082a3683
@ -1786,6 +1786,55 @@ delete_variant_cb (GObject *object,
|
|||||||
g_hash_table_remove (global->save_ops, 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
|
static void
|
||||||
replace_variant_cb (GObject *object,
|
replace_variant_cb (GObject *object,
|
||||||
GAsyncResult *result,
|
GAsyncResult *result,
|
||||||
@ -1794,7 +1843,7 @@ replace_variant_cb (GObject *object,
|
|||||||
ShellGlobal *global = user_data;
|
ShellGlobal *global = user_data;
|
||||||
GError *error = NULL;
|
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))
|
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
|
||||||
{
|
{
|
||||||
@ -1830,12 +1879,19 @@ save_variant (ShellGlobal *global,
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
g_file_replace_contents_async (path,
|
g_autoptr(GBytes) bytes = NULL;
|
||||||
g_variant_get_data (variant),
|
|
||||||
|
bytes = g_bytes_new_with_free_func (g_variant_get_data (variant),
|
||||||
g_variant_get_size (variant),
|
g_variant_get_size (variant),
|
||||||
NULL, FALSE,
|
(GDestroyNotify)g_variant_unref,
|
||||||
G_FILE_CREATE_REPLACE_DESTINATION,
|
g_variant_ref (variant));
|
||||||
cancellable, replace_variant_cb, global);
|
/* 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
|
||||||
|
* compositor 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);
|
g_object_unref (path);
|
||||||
|
Loading…
Reference in New Issue
Block a user