From ef4b43ff1301067bc73eb4a07191e2489c6e9fe3 Mon Sep 17 00:00:00 2001 From: Nishal Kulkarni Date: Wed, 24 Nov 2021 16:22:28 +0530 Subject: [PATCH] gutils: Make g_set_prgname() thread-safe Currently `g_prgname` can be freed by `g_set_prgname()` while another thread is holding a pointer to it. We use GQuark when setting g_prgname so that string is never released once set. Also added unit test, which checks if setting prgname in multi-threaded program is safe. Closes: #847 --- glib/gutils.c | 13 ++++++++----- glib/tests/utils.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/glib/gutils.c b/glib/gutils.c index b744f2510..872b436ed 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -69,6 +69,7 @@ #include "garray.h" #include "glibintl.h" #include "gstdio.h" +#include "gquark.h" #ifdef G_PLATFORM_WIN32 #include "gconvert.h" @@ -1050,7 +1051,7 @@ g_get_host_name (void) } G_LOCK_DEFINE_STATIC (g_prgname); -static gchar *g_prgname = NULL; +static const gchar *g_prgname = NULL; /* always a quark */ /** * g_get_prgname: @@ -1071,7 +1072,7 @@ static gchar *g_prgname = NULL; const gchar* g_get_prgname (void) { - gchar* retval; + const gchar* retval; G_LOCK (g_prgname); retval = g_prgname; @@ -1093,14 +1094,16 @@ g_get_prgname (void) * #GtkApplication::startup handler. The program name is found by * taking the last component of @argv[0]. * - * Note that for thread-safety reasons this function can only be called once. + * Since GLib 2.72, this function can be called multiple times + * and is fully thread safe. Prior to GLib 2.72, this function + * could only be called once per process. */ void g_set_prgname (const gchar *prgname) { + GQuark qprgname = g_quark_from_string (prgname); G_LOCK (g_prgname); - g_free (g_prgname); - g_prgname = g_strdup (prgname); + g_prgname = g_quark_to_string (qprgname); G_UNLOCK (g_prgname); } diff --git a/glib/tests/utils.c b/glib/tests/utils.c index f47e3595c..6b71e9cbb 100644 --- a/glib/tests/utils.c +++ b/glib/tests/utils.c @@ -158,6 +158,52 @@ test_appname (void) g_assert_cmpstr (appname, ==, "appname"); } +static gpointer +thread_prgname_check (gpointer data) +{ + gint *n_threads_got_prgname = (gint *) data; + const gchar *old_prgname; + + old_prgname = g_get_prgname (); + g_assert_cmpstr (old_prgname, ==, "prgname"); + + g_atomic_int_inc (n_threads_got_prgname); + + while (g_strcmp0 (g_get_prgname (), "prgname2") != 0); + + return NULL; +} + +static void +test_prgname_thread_safety (void) +{ + gsize i; + gint n_threads_got_prgname; + GThread *threads[4]; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/847"); + g_test_summary ("Test that threads racing to get and set the program name " + "always receive a valid program name."); + + g_set_prgname ("prgname"); + g_atomic_int_set (&n_threads_got_prgname, 0); + + for (i = 0; i < G_N_ELEMENTS (threads); i++) + threads[i] = g_thread_new (NULL, thread_prgname_check, &n_threads_got_prgname); + + while (g_atomic_int_get (&n_threads_got_prgname) != G_N_ELEMENTS (threads)) + g_usleep (50); + + g_set_prgname ("prgname2"); + + /* Wait for all the workers to exit. */ + for (i = 0; i < G_N_ELEMENTS (threads); i++) + g_thread_join (threads[i]); + + /* reset prgname */ + g_set_prgname ("prgname"); +} + static void test_tmpdir (void) { @@ -860,6 +906,7 @@ main (int argc, g_test_add_func ("/utils/locale-variants", test_locale_variants); g_test_add_func ("/utils/version", test_version); g_test_add_func ("/utils/appname", test_appname); + g_test_add_func ("/utils/prgname-thread-safety", test_prgname_thread_safety); g_test_add_func ("/utils/tmpdir", test_tmpdir); g_test_add_func ("/utils/bits", test_bits); g_test_add_func ("/utils/swap", test_swap);