From 167deee94ddf5ca8e88b3bb17d4275728a7d1287 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 8 Nov 2023 08:27:20 +0100 Subject: [PATCH 1/4] gutils: use atomic pointers for g_prgname The mutex is not necessary to guard a single pointer. --- glib/gutils.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/glib/gutils.c b/glib/gutils.c index 5a15e99b1..0ec4f66a1 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -1128,7 +1128,6 @@ g_get_host_name (void) return hostname; } -G_LOCK_DEFINE_STATIC (g_prgname); static const gchar *g_prgname = NULL; /* always a quark */ /** @@ -1150,13 +1149,7 @@ static const gchar *g_prgname = NULL; /* always a quark */ const gchar* g_get_prgname (void) { - const gchar* retval; - - G_LOCK (g_prgname); - retval = g_prgname; - G_UNLOCK (g_prgname); - - return retval; + return g_atomic_pointer_get (&g_prgname); } /** @@ -1179,10 +1172,8 @@ g_get_prgname (void) void g_set_prgname (const gchar *prgname) { - GQuark qprgname = g_quark_from_string (prgname); - G_LOCK (g_prgname); - g_prgname = g_quark_to_string (qprgname); - G_UNLOCK (g_prgname); + prgname = g_intern_string (prgname); + g_atomic_pointer_set (&g_prgname, prgname); } G_LOCK_DEFINE_STATIC (g_application_name); From 0d61895ef1b4cf7a706841681322f53abc494543 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 8 Nov 2023 08:31:10 +0100 Subject: [PATCH 2/4] gutils: assert against calling g_set_application_name() g_set_application_name() guards against being reset, but it doesn't remember whether it was set, it only checks whether g_application_name was set to non-NULL. When allowing g_set_application_name(NULL) that leads to odd behaviors, like: g_set_application_name(NULL); g_set_application_name("foo"); would not warn. Disallow that and assert against a NULL application_name. Note that application_name argument is also not marked as "(nullable)". --- glib/gutils.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/glib/gutils.c b/glib/gutils.c index 0ec4f66a1..58b87fe84 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -1232,7 +1232,9 @@ void g_set_application_name (const gchar *application_name) { gboolean already_set = FALSE; - + + g_return_if_fail (application_name); + G_LOCK (g_application_name); if (g_application_name) already_set = TRUE; From 54e0b2d75bf0129af361ffef931c2f8e9c820e57 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 8 Nov 2023 08:27:20 +0100 Subject: [PATCH 3/4] gutils: use atomic pointers for g_application_name The mutex is not necessary to guard a single pointer. --- glib/gutils.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/glib/gutils.c b/glib/gutils.c index 58b87fe84..f26b3a76b 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -1176,7 +1176,6 @@ g_set_prgname (const gchar *prgname) g_atomic_pointer_set (&g_prgname, prgname); } -G_LOCK_DEFINE_STATIC (g_application_name); static gchar *g_application_name = NULL; /** @@ -1198,16 +1197,14 @@ static gchar *g_application_name = NULL; const gchar * g_get_application_name (void) { - gchar* retval; + const char *retval; - G_LOCK (g_application_name); - retval = g_application_name; - G_UNLOCK (g_application_name); + retval = g_atomic_pointer_get (&g_application_name); - if (retval == NULL) - return g_get_prgname (); - - return retval; + if (retval) + return retval; + + return g_get_prgname (); } /** @@ -1231,19 +1228,17 @@ g_get_application_name (void) void g_set_application_name (const gchar *application_name) { - gboolean already_set = FALSE; + char *name; g_return_if_fail (application_name); - G_LOCK (g_application_name); - if (g_application_name) - already_set = TRUE; - else - g_application_name = g_strdup (application_name); - G_UNLOCK (g_application_name); + name = g_strdup (application_name); - if (already_set) - g_warning ("g_set_application_name() called multiple times"); + if (!g_atomic_pointer_compare_and_exchange (&g_application_name, NULL, name)) + { + g_warning ("g_set_application_name() called multiple times"); + g_free (name); + } } #ifdef G_OS_WIN32 From 7098250e7aa2f4aef2775f71f31c4e9642dad768 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 8 Nov 2023 08:48:35 +0100 Subject: [PATCH 4/4] gutils: avoid race setting prgname from g_option_context_parse()/g_application_run() g_option_context_parse()/g_application_run()/g_test_init() for convenience also call g_set_prgname(), when the prgname is unset at this point. This was racy. Fix the race by using an atomic compare-and-exchange and only reset the value, if it is unset still. --- gio/gapplication.c | 3 ++- glib/glib-private.c | 3 +++ glib/glib-private.h | 3 +++ glib/goption.c | 6 ++---- glib/gtestutils.c | 4 ++-- glib/gutils.c | 19 +++++++++++++++++++ glib/gutilsprivate.h | 2 ++ 7 files changed, 33 insertions(+), 7 deletions(-) diff --git a/gio/gapplication.c b/gio/gapplication.c index 210ebaf6e..7465c9a54 100644 --- a/gio/gapplication.c +++ b/gio/gapplication.c @@ -38,6 +38,7 @@ #include "gioenumtypes.h" #include "gioenums.h" #include "gfile.h" +#include "glib-private.h" #include "glibintl.h" #include "gmarshal-internal.h" @@ -2526,7 +2527,7 @@ g_application_run (GApplication *application, gchar *prgname; prgname = g_path_get_basename (argv[0]); - g_set_prgname (prgname); + GLIB_PRIVATE_CALL (g_set_prgname_once) (prgname); g_free (prgname); } diff --git a/glib/glib-private.c b/glib/glib-private.c index 9f4887aa2..c2b68a060 100644 --- a/glib/glib-private.c +++ b/glib/glib-private.c @@ -23,6 +23,7 @@ #include "glib-private.h" #include "glib-init.h" +#include "gutilsprivate.h" #ifdef USE_INVALID_PARAMETER_HANDLER #include @@ -71,6 +72,8 @@ glib__private__ (void) g_find_program_for_path, g_uri_get_default_scheme_port, + + g_set_prgname_once, }; return &table; diff --git a/glib/glib-private.h b/glib/glib-private.h index e96a73fb6..50aa8a050 100644 --- a/glib/glib-private.h +++ b/glib/glib-private.h @@ -288,6 +288,9 @@ typedef struct { /* See guri.c */ int (* g_uri_get_default_scheme_port) (const char *scheme); + /* See gutils.c */ + gboolean (* g_set_prgname_once) (const gchar *prgname); + /* Add other private functions here, initialize them in glib-private.c */ } GLibPrivateVTable; diff --git a/glib/goption.c b/glib/goption.c index e6d8bf1b8..781c5bae6 100644 --- a/glib/goption.c +++ b/glib/goption.c @@ -35,6 +35,7 @@ #include "gprintf.h" #include "glibintl.h" +#include "gutilsprivate.h" #if defined G_OS_WIN32 #include @@ -1813,10 +1814,7 @@ g_option_context_parse (GOptionContext *context, else prgname = platform_get_argv0 (); - if (prgname) - g_set_prgname (prgname); - else - g_set_prgname (""); + g_set_prgname_once (prgname ? prgname : "