[glib: 1/2] gutils: Make g_set_prgname() thread-safe




commit ef4b43ff1301067bc73eb4a07191e2489c6e9fe3
Author: Nishal Kulkarni <nishalkulkarni gmail com>
Date:   Wed Nov 24 16:22:28 2021 +0530

    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);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]