Merge branch 'gtask-must-have-result' into 'main'

gtask: Warn if a GTask is finalised without returning

See merge request GNOME/glib!385

3 files changed, 72 insertions, 0 deletions

diff --git a/gio/gtask.c b/gio/gtask.c
index 1d5b37e85..afa1b613d 100644
--- a/gio/gtask.c
+++ b/gio/gtask.c
@@ -64,6 +64,10 @@
  * #GTask was constructed to be running at least until the task has completed
  * and its data has been freed.
  *
+ * If a #GTask has been constructed and its callback set, it is an error to
+ * not call `g_task_return_*()` on it. GLib will warn at runtime if this happens
+ * (since 2.76).
+ *
  * Here is an example for using GTask as a GAsyncResult:
  * |[<!-- language="C" -->
  *     typedef struct {
@@ -659,6 +663,34 @@ g_task_finalize (GObject *object)
 {
   GTask *task = G_TASK (object);
 
+  /* Warn if a #GTask is finalised without g_task_return() ever having been
+   * called on it.
+   *
+   * Tasks without a callback or which are run in g_task_run_in_thread{,_sync}()
+   * only trigger a debug message as that’s sometimes used as a pattern for
+   * running work in a worker thread without caring about the result. */
+  if (!task->ever_returned)
+    {
+      gchar *owned_task_name = NULL;
+      const gchar *task_name = g_task_get_name (task);
+
+      if (task_name == NULL)
+        task_name = owned_task_name = g_strdup_printf ("%p", task);
+
+      if (task->callback != NULL && !G_TASK_IS_THREADED (task))
+        g_critical ("GTask %s (source object: %p, source tag: %p) finalized without "
+                    "ever returning (using g_task_return_*()). This potentially "
+                    "indicates a bug in the program.",
+                    task_name, task->source_object, task->source_tag);
+      else
+        g_debug ("GTask %s (source object: %p, source tag: %p) finalized without "
+                 "ever returning (using g_task_return_*()). This potentially "
+                 "indicates a bug in the program.",
+                 task_name, task->source_object, task->source_tag);
+
+      g_free (owned_task_name);
+    }
+
   g_clear_object (&task->source_object);
   g_clear_object (&task->cancellable);
   if (!task->name_is_static)
diff --git a/gio/tests/gdbus-server-auth.c b/gio/tests/gdbus-server-auth.c
index c683f61da..319a9948b 100644
--- a/gio/tests/gdbus-server-auth.c
+++ b/gio/tests/gdbus-server-auth.c
@@ -210,6 +210,7 @@ libdbus_call_task_cb (GTask *task,
                                                                    libdbus_call->call,
                                                                    -1,
                                                                    &libdbus_call->error);
+  g_task_return_boolean (task, TRUE);
 }
 #endif /* HAVE_DBUS1 */
 
diff --git a/gio/tests/task.c b/gio/tests/task.c
index 6fe35fb21..8dfc0e983 100644
--- a/gio/tests/task.c
+++ b/gio/tests/task.c
@@ -2451,6 +2451,44 @@ test_attach_source_set_name (void)
   g_object_unref (task);
 }
 
+static void
+test_finalize_without_return (void)
+{
+  GTask *task = NULL;
+  guint n_calls = 0;
+
+  /* With a callback set. */
+  task = g_task_new (NULL, NULL, task_complete_cb, &n_calls);
+
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                         "GTask * (source object: *, source tag: *) finalized without "
+                         "ever returning (using g_task_return_*()). This potentially "
+                         "indicates a bug in the program.");
+  g_object_unref (task);
+  g_test_assert_expected_messages ();
+
+  /* With a callback and task name set. */
+  task = g_task_new (NULL, NULL, task_complete_cb, &n_calls);
+  g_task_set_static_name (task, "oogly boogly");
+
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                         "GTask oogly boogly (source object: *, source tag: *) finalized without "
+                         "ever returning (using g_task_return_*()). This potentially "
+                         "indicates a bug in the program.");
+  g_object_unref (task);
+  g_test_assert_expected_messages ();
+
+  /* Without a callback set. */
+  task = g_task_new (NULL, NULL, NULL, NULL);
+
+  g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG,
+                         "GTask * (source object: *, source tag: *) finalized without "
+                         "ever returning (using g_task_return_*()). This potentially "
+                         "indicates a bug in the program.");
+  g_object_unref (task);
+  g_test_assert_expected_messages ();
+}
+
 int
 main (int argc, char **argv)
 {
@@ -2494,6 +2532,7 @@ main (int argc, char **argv)
   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);
+  g_test_add_func ("/gtask/finalize-without-return", test_finalize_without_return);
 
   ret = g_test_run();