Merge branch 'task-strdup-avoidance' into 'main'

task: Don't overwrite source names

See merge request GNOME/glib!2869

docs/reference/gio/gio-sections-common.txt

@@ -4562,6 +4562,7 @@ g_task_set_check_cancellable
 g_task_set_return_on_cancel
 g_task_set_source_tag
 g_task_set_name
+g_task_set_static_name
 <SUBSECTION>
 g_task_report_error
 g_task_report_new_error

gio/gdbusconnection.c

@@ -1945,6 +1945,7 @@ g_dbus_connection_send_message_with_reply_unlocked (GDBusConnection     *connect
   if (timeout_msec != G_MAXINT)
     {
       data->timeout_source = g_timeout_source_new (timeout_msec);
+      g_source_set_static_name (data->timeout_source, "[gio] send_message_with_reply_unlocked");
       g_task_attach_source (task, data->timeout_source,
                             (GSourceFunc) send_message_with_reply_timeout_cb);
       g_source_unref (data->timeout_source);

gio/gtask.c

@@ -588,6 +588,7 @@ struct _GTask {
   guint check_cancellable : 1;
   guint synchronous : 1;
   guint blocking_other_task : 1;
+  guint name_is_static : 1;
 
   GError *error;
   union {
@@ -660,7 +661,8 @@ g_task_finalize (GObject *object)
 
   g_clear_object (&task->source_object);
   g_clear_object (&task->cancellable);
-  g_free (task->name);
+  if (!task->name_is_static)
+    g_free (task->name);
 
   if (task->context)
     g_main_context_unref (task->context);
@@ -769,7 +771,7 @@ g_task_report_error (gpointer             source_object,
 
   task = g_task_new (source_object, NULL, callback, callback_data);
   g_task_set_source_tag (task, source_tag);
-  g_task_set_name (task, G_STRFUNC);
+  g_task_set_static_name (task, G_STRFUNC);
   g_task_return_error (task, error);
   g_object_unref (task);
 }
@@ -1028,16 +1030,41 @@ void
  * Since: 2.60
  */
 void
-g_task_set_name (GTask       *task,
-                 const gchar *name)
+(g_task_set_name) (GTask      *task,
+                   const char *name)
 {
-  gchar *new_name;
+  char *new_name;
 
   g_return_if_fail (G_IS_TASK (task));
 
   new_name = g_strdup (name);
-  g_free (task->name);
+  if (!task->name_is_static)
+    g_free (task->name);
   task->name = g_steal_pointer (&new_name);
+  task->name_is_static = FALSE;
+}
+
+/**
+ * g_task_set_static_name:
+ * @task: a #GTask
+ * @name: (nullable): a human readable name for the task. Must be a string literal
+ *
+ * Sets @task’s name, used in debugging and profiling.
+ *
+ * This is a variant of g_task_set_name() that avoids copying @name.
+ *
+ * Since: 2.76
+ */
+void
+g_task_set_static_name (GTask      *task,
+                        const char *name)
+{
+  g_return_if_fail (G_IS_TASK (task));
+
+  if (!task->name_is_static)
+    g_free (task->name);
+  task->name = (char *) name;
+  task->name_is_static = TRUE;
 }
 
 /**
@@ -1645,7 +1672,8 @@ g_task_run_in_thread_sync (GTask           *task,
  * callback to @callback, with @task as the callback's `user_data`.
  *
  * It will set the @source’s name to the task’s name (as set with
- * g_task_set_name()), if one has been set.
+ * g_task_set_name()), if one has been set on the task and the source doesn’t
+ * yet have a name.
  *
  * This takes a reference on @task until @source is destroyed.
  *
@@ -1661,7 +1689,7 @@ g_task_attach_source (GTask       *task,
   g_source_set_callback (source, callback,
                          g_object_ref (task), g_object_unref);
   g_source_set_priority (source, task->priority);
-  if (task->name != NULL)
+  if (task->name != NULL && g_source_get_name (source) == NULL)
     g_source_set_name (source, task->name);
 
   g_source_attach (source, task->context);

gio/gtask.h

@@ -79,6 +79,9 @@ void          g_task_set_source_tag        (GTask               *task,
 GIO_AVAILABLE_IN_2_60
 void          g_task_set_name              (GTask               *task,
                                             const gchar         *name);
+GIO_AVAILABLE_IN_2_76
+void          g_task_set_static_name       (GTask               *task,
+                                            const gchar         *name);
 
 /* Macro wrapper to set the task name when setting the source tag. */
 #if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_60
@@ -86,10 +89,22 @@ void          g_task_set_name              (GTask               *task,
   GTask *_task = (task); \
   (g_task_set_source_tag) (_task, tag); \
   if (g_task_get_name (_task) == NULL) \
-    g_task_set_name (_task, G_STRINGIFY (tag)); \
+    g_task_set_static_name (_task, G_STRINGIFY (tag)); \
 } G_STMT_END
 #endif
 
+#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_76
+#if defined (__GNUC__) && (__GNUC__ >= 2)
+#define g_task_set_name(task, name) G_STMT_START { \
+  GTask *_task = (task); \
+  if (__builtin_constant_p (name)) \
+    g_task_set_static_name (_task, name); \
+  else \
+    g_task_set_name (_task, name); \
+} G_STMT_END
+#endif
+#endif
+
 GIO_AVAILABLE_IN_2_36
 gpointer      g_task_get_source_object     (GTask               *task);
 GIO_AVAILABLE_IN_2_36

gio/tests/task.c

@@ -625,18 +625,40 @@ static void
 test_name (void)
 {
   GTask *t1 = NULL;
+  char *orig = g_strdup ("some task");
   gchar *name1 = NULL;
 
   t1 = g_task_new (NULL, NULL, name_callback, &name1);
-  g_task_set_name (t1, "some task");
+  (g_task_set_name) (t1, orig);
   g_task_return_boolean (t1, TRUE);
   g_object_unref (t1);
 
   g_main_loop_run (loop);
 
-  g_assert_cmpstr (name1, ==, "some task");
+  g_assert_cmpstr (name1, ==, orig);
 
   g_free (name1);
+  g_free (orig);
+}
+
+static void
+test_name_macro_wrapper (void)
+{
+  GTask *t1 = NULL;
+  char *orig = g_strdup ("some task");
+  gchar *name1 = NULL;
+
+  t1 = g_task_new (NULL, NULL, name_callback, &name1);
+  g_task_set_name (t1, orig);
+  g_task_return_boolean (t1, TRUE);
+  g_object_unref (t1);
+
+  g_main_loop_run (loop);
+
+  g_assert_cmpstr (name1, ==, orig);
+
+  g_free (name1);
+  g_free (orig);
 }
 
 static void
@@ -656,6 +678,44 @@ name_callback (GObject      *object,
   g_main_loop_quit (loop);
 }
 
+static void static_name_callback (GObject      *object,
+                                  GAsyncResult *result,
+                                  gpointer      user_data);
+
+static void
+test_static_name (void)
+{
+  GTask *t1 = NULL;
+  char *orig = "some task";
+  char *name1 = NULL;
+
+  t1 = g_task_new (NULL, NULL, static_name_callback, &name1);
+  g_task_set_static_name (t1, orig);
+  g_task_return_boolean (t1, TRUE);
+  g_object_unref (t1);
+
+  g_main_loop_run (loop);
+
+  g_assert_true (name1 == orig);
+}
+
+static void
+static_name_callback (GObject      *object,
+                      GAsyncResult *result,
+                      gpointer      user_data)
+{
+  const char **name_out = user_data;
+  GError *local_error = NULL;
+
+  g_assert_null (*name_out);
+  *name_out = g_task_get_name (G_TASK (result));
+
+  g_task_propagate_boolean (G_TASK (result), &local_error);
+  g_assert_no_error (local_error);
+
+  g_main_loop_quit (loop);
+}
+
 /* test_asynchronous_cancellation: cancelled tasks are returned
  * asynchronously, i.e. not from inside the GCancellable::cancelled
  * handler.
@@ -2358,6 +2418,39 @@ test_return_value_first (void)
   g_test_trap_assert_stderr ("*CRITICAL*assertion '!task->ever_returned' failed*");
 }
 
+static gboolean
+source_cb (gpointer user_data)
+{
+  return G_SOURCE_REMOVE;
+}
+
+static void
+test_attach_source_set_name (void)
+{
+  guint calls = 0;
+  GTask *task = NULL;
+  GSource *source = NULL;
+  GSourceFuncs source_funcs = { NULL, NULL, NULL, NULL, NULL, NULL };
+
+  g_test_summary ("Test that attaching a source to a task will set the source’s name if unset");
+
+  task = g_task_new (NULL, NULL, task_complete_cb, &calls);
+  g_task_set_name (task, "test name");
+
+  source = g_source_new (&source_funcs, sizeof (GSource));
+  g_task_attach_source (task, source, source_cb);
+  g_assert_cmpstr (g_source_get_name (source), ==, "test name");
+  g_source_unref (source);
+
+  source = g_source_new (&source_funcs, sizeof (GSource));
+  g_source_set_name (source, "not the task name");
+  g_task_attach_source (task, source, source_cb);
+  g_assert_cmpstr (g_source_get_name (source), ==, "not the task name");
+  g_source_unref (source);
+
+  g_object_unref (task);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -2379,6 +2472,8 @@ main (int argc, char **argv)
   g_test_add_func ("/gtask/report-error", test_report_error);
   g_test_add_func ("/gtask/priority", test_priority);
   g_test_add_func ("/gtask/name", test_name);
+  g_test_add_func ("/gtask/name/macro-wrapper", test_name_macro_wrapper);
+  g_test_add_func ("/gtask/static-name", test_static_name);
   g_test_add_func ("/gtask/asynchronous-cancellation", test_asynchronous_cancellation);
   g_test_add_func ("/gtask/check-cancellable", test_check_cancellable);
   g_test_add_func ("/gtask/return-if-cancelled", test_return_if_cancelled);
@@ -2398,6 +2493,7 @@ main (int argc, char **argv)
   g_test_add_func ("/gtask/return/in-idle/value-first", test_return_in_idle_value_first);
   g_test_add_func ("/gtask/return/error-first", test_return_error_first);
   g_test_add_func ("/gtask/return/value-first", test_return_value_first);
+  g_test_add_func ("/gtask/attach-source/set-name", test_attach_source_set_name);
 
   ret = g_test_run();