From 5bf032a5d19748c7e04ae0562f1e3123eb8ca3b9 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Tue, 28 Jul 2020 13:09:27 +0100 Subject: [PATCH] all: Consistently work around glib!490 As noted in Flatpak and in GNOME/glib!490, there is a bug in GLib < 2.60 where g_spawn_* can sometimes deadlock while closing fds in a multi-threaded application. Work around this by making the affected fds close-on-execute ourselves, much like Flatpak does. Signed-off-by: Simon McVittie <smcv@collabora.com> --- src/adverb.c | 20 ++++++++++++++++---- src/bwrap.c | 16 ++++++++-------- src/utils.c | 13 +++++++++++-- src/wrap.c | 6 ++++-- 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/adverb.c b/src/adverb.c index 508424216..571c01624 100644 --- a/src/adverb.c +++ b/src/adverb.c @@ -39,6 +39,7 @@ #include "libglnx/libglnx.h" #include "bwrap-lock.h" +#include "flatpak-utils-base-private.h" #include "utils.h" #ifndef PR_SET_CHILD_SUBREAPER @@ -54,6 +55,12 @@ static gboolean opt_version = FALSE; static gboolean opt_wait = FALSE; static gboolean opt_write = FALSE; +static void +child_setup_cb (gpointer user_data) +{ + flatpak_close_fds_workaround (3); +} + static gboolean opt_fd_cb (const char *name, const char *value, @@ -166,11 +173,13 @@ generate_locales (gchar **locpath_out, NULL }; + /* We use LEAVE_DESCRIPTORS_OPEN to work around a deadlock in older GLib, + * see flatpak_close_fds_workaround */ if (!g_spawn_sync (NULL, /* cwd */ (gchar **) locale_gen_argv, NULL, /* environ */ - G_SPAWN_DEFAULT, - NULL, NULL, /* child setup */ + G_SPAWN_LEAVE_DESCRIPTORS_OPEN, + child_setup_cb, NULL, &child_stdout, &child_stderr, &wait_status, @@ -421,11 +430,14 @@ main (int argc, g_debug ("Launching child process..."); + /* We use LEAVE_DESCRIPTORS_OPEN to work around a deadlock in older GLib, + * see flatpak_close_fds_workaround */ if (!g_spawn_async (NULL, /* working directory */ command_and_args, my_environ, - G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD, - NULL, NULL, /* child setup + user_data */ + (G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD | + G_SPAWN_LEAVE_DESCRIPTORS_OPEN), + child_setup_cb, NULL, &child_pid, &local_error)) { diff --git a/src/bwrap.c b/src/bwrap.c index 437ea68fc..bdaec7d5d 100644 --- a/src/bwrap.c +++ b/src/bwrap.c @@ -45,8 +45,7 @@ pv_bwrap_run_sync (FlatpakBwrap *bwrap, g_autofree gchar *errors = NULL; guint i; g_autoptr(GString) command = g_string_new (""); - void (*child_setup) (gpointer) = NULL; - gpointer child_setup_data = NULL; + GArray *child_setup_data = NULL; g_return_val_if_fail (bwrap != NULL, FALSE); g_return_val_if_fail (bwrap->argv->len >= 2, FALSE); @@ -69,18 +68,19 @@ pv_bwrap_run_sync (FlatpakBwrap *bwrap, } if (bwrap->fds != NULL && bwrap->fds->len > 0) - { - child_setup = flatpak_bwrap_child_setup_cb; - child_setup_data = bwrap->fds; - } + child_setup_data = bwrap->fds; g_debug ("run:%s", command->str); + /* We use LEAVE_DESCRIPTORS_OPEN to work around a deadlock in older GLib, + * see flatpak_close_fds_workaround */ if (!g_spawn_sync (NULL, /* cwd */ (char **) bwrap->argv->pdata, bwrap->envp, - G_SPAWN_SEARCH_PATH, - child_setup, child_setup_data, + (G_SPAWN_SEARCH_PATH | + G_SPAWN_LEAVE_DESCRIPTORS_OPEN), + flatpak_bwrap_child_setup_cb, + child_setup_data, &output, &errors, &exit_status, diff --git a/src/utils.c b/src/utils.c index b299b3c0c..e68282d8e 100644 --- a/src/utils.c +++ b/src/utils.c @@ -35,6 +35,12 @@ #include "flatpak-utils-base-private.h" #include "flatpak-utils-private.h" +static void +child_setup_cb (gpointer user_data) +{ + flatpak_close_fds_workaround (3); +} + /** * pv_avoid_gvfs: * @@ -228,11 +234,14 @@ pv_capture_output (const char * const * argv, g_debug ("run:%s", command->str); + /* We use LEAVE_DESCRIPTORS_OPEN to work around a deadlock in older GLib, + * see flatpak_close_fds_workaround */ if (!g_spawn_sync (NULL, /* cwd */ (char **) argv, NULL, /* env */ - G_SPAWN_SEARCH_PATH, - NULL, NULL, /* child setup */ + (G_SPAWN_SEARCH_PATH | + G_SPAWN_LEAVE_DESCRIPTORS_OPEN), + child_setup_cb, NULL, &output, &errors, &wait_status, diff --git a/src/wrap.c b/src/wrap.c index c3a55fe68..1eabcd016 100644 --- a/src/wrap.c +++ b/src/wrap.c @@ -136,11 +136,13 @@ check_bwrap (const char *tools_dir, bwrap_test_argv[0] = bwrap_executable; + /* We use LEAVE_DESCRIPTORS_OPEN to work around a deadlock in older GLib, + * see flatpak_close_fds_workaround */ if (!g_spawn_sync (NULL, /* cwd */ (gchar **) bwrap_test_argv, NULL, /* environ */ - G_SPAWN_DEFAULT, - NULL, NULL, /* child setup */ + G_SPAWN_LEAVE_DESCRIPTORS_OPEN, + flatpak_bwrap_child_setup_cb, NULL, &child_stdout, &child_stderr, &wait_status, -- GitLab