diff options
author | Philip Withnall <philip@tecnocode.co.uk> | 2018-10-02 10:07:49 +0000 |
---|---|---|
committer | Philip Withnall <philip@tecnocode.co.uk> | 2018-10-02 10:07:49 +0000 |
commit | 5002d87dece1f3f602a952369ee0bfb070830999 (patch) | |
tree | 17ddd86425fd3975306eba07b262042288d095f8 | |
parent | 32e049b761d7ea2b9785a3428cf3b60119d14ed9 (diff) | |
parent | 290bb0dd1b5169806e7de2a17ad22a3432661d31 (diff) | |
download | glib-5002d87dece1f3f602a952369ee0bfb070830999.tar.gz |
Merge branch '1525-task-result' into 'master'
gtask: Check an error hasn’t been returned when calling g_task_return*()
Closes #1525
See merge request GNOME/glib!368
-rw-r--r-- | gio/gtask.c | 39 | ||||
-rw-r--r-- | gio/tests/task.c | 137 |
2 files changed, 159 insertions, 17 deletions
diff --git a/gio/gtask.c b/gio/gtask.c index 4087543e6..a40bc01b4 100644 --- a/gio/gtask.c +++ b/gio/gtask.c @@ -551,20 +551,26 @@ struct _GTask { gint64 creation_time; gint priority; GCancellable *cancellable; - gboolean check_cancellable; GAsyncReadyCallback callback; gpointer callback_data; - gboolean completed; GTaskThreadFunc task_func; GMutex lock; GCond cond; - gboolean return_on_cancel; + + /* This can’t be in the bit field because we access it from TRACE(). */ gboolean thread_cancelled; - gboolean synchronous; - gboolean thread_complete; - gboolean blocking_other_task; + + gboolean check_cancellable : 1; + gboolean completed : 1; + gboolean return_on_cancel : 1; + gboolean synchronous : 1; + gboolean thread_complete : 1; + gboolean blocking_other_task : 1; + gboolean had_error : 1; + gboolean result_set : 1; + gboolean ever_returned : 1; GError *error; union { @@ -573,8 +579,6 @@ struct _GTask { gboolean boolean; } result; GDestroyNotify result_destroy; - gboolean had_error; - gboolean result_set; }; #define G_TASK_IS_THREADED(task) ((task)->task_func != NULL) @@ -1176,6 +1180,9 @@ g_task_return (GTask *task, { GSource *source; + if (type != G_TASK_RETURN_FROM_THREAD) + task->ever_returned = TRUE; + if (type == G_TASK_RETURN_SUCCESS) task->result_set = TRUE; @@ -1596,7 +1603,7 @@ g_task_return_pointer (GTask *task, GDestroyNotify result_destroy) { g_return_if_fail (G_IS_TASK (task)); - g_return_if_fail (task->result_set == FALSE); + g_return_if_fail (!task->ever_returned); task->result.pointer = result; task->result_destroy = result_destroy; @@ -1631,7 +1638,7 @@ g_task_propagate_pointer (GTask *task, if (g_task_propagate_error (task, error)) return NULL; - g_return_val_if_fail (task->result_set == TRUE, NULL); + g_return_val_if_fail (task->result_set, NULL); task->result_destroy = NULL; task->result_set = FALSE; @@ -1654,7 +1661,7 @@ g_task_return_int (GTask *task, gssize result) { g_return_if_fail (G_IS_TASK (task)); - g_return_if_fail (task->result_set == FALSE); + g_return_if_fail (!task->ever_returned); task->result.size = result; @@ -1687,7 +1694,7 @@ g_task_propagate_int (GTask *task, if (g_task_propagate_error (task, error)) return -1; - g_return_val_if_fail (task->result_set == TRUE, -1); + g_return_val_if_fail (task->result_set, -1); task->result_set = FALSE; return task->result.size; @@ -1709,7 +1716,7 @@ g_task_return_boolean (GTask *task, gboolean result) { g_return_if_fail (G_IS_TASK (task)); - g_return_if_fail (task->result_set == FALSE); + g_return_if_fail (!task->ever_returned); task->result.boolean = result; @@ -1742,7 +1749,7 @@ g_task_propagate_boolean (GTask *task, if (g_task_propagate_error (task, error)) return FALSE; - g_return_val_if_fail (task->result_set == TRUE, FALSE); + g_return_val_if_fail (task->result_set, FALSE); task->result_set = FALSE; return task->result.boolean; @@ -1772,7 +1779,7 @@ g_task_return_error (GTask *task, GError *error) { g_return_if_fail (G_IS_TASK (task)); - g_return_if_fail (task->result_set == FALSE); + g_return_if_fail (!task->ever_returned); g_return_if_fail (error != NULL); task->error = error; @@ -1833,7 +1840,7 @@ g_task_return_error_if_cancelled (GTask *task) GError *error = NULL; g_return_val_if_fail (G_IS_TASK (task), FALSE); - g_return_val_if_fail (task->result_set == FALSE, FALSE); + g_return_val_if_fail (!task->ever_returned, FALSE); if (g_cancellable_set_error_if_cancelled (task->cancellable, &error)) { diff --git a/gio/tests/task.c b/gio/tests/task.c index 934262e40..7f698306b 100644 --- a/gio/tests/task.c +++ b/gio/tests/task.c @@ -1689,7 +1689,6 @@ test_return_on_cancel_atomic (void) g_task_set_task_data (task, (gpointer)&state, NULL); g_task_run_in_thread (task, return_on_cancel_atomic_thread); - g_object_unref (task); g_assert_cmpint (state, ==, 0); @@ -1724,6 +1723,7 @@ test_return_on_cancel_atomic (void) g_object_unref (cancellable); g_mutex_unlock (&roca_mutex_1); g_mutex_unlock (&roca_mutex_2); + g_object_unref (task); } /* test_return_pointer: memory management of pointer returns */ @@ -1987,6 +1987,136 @@ test_legacy_error (void) g_assert (simple == NULL); } +/* Various helper functions for the return tests below. */ +static void +task_complete_cb (GObject *source, + GAsyncResult *result, + gpointer user_data) +{ + GTask *task = G_TASK (result); + guint *calls = user_data; + + g_assert_cmpint (++*calls, <=, 1); + + /* Propagate the result, so it’s removed from the task’s internal state. */ + g_task_propagate_boolean (task, NULL); +} + +static void +return_twice (GTask *task) +{ + gboolean error_first = GPOINTER_TO_UINT (g_task_get_task_data (task)); + + if (error_first) + { + g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_UNKNOWN, "oh no"); + g_task_return_boolean (task, TRUE); + } + else + { + g_task_return_boolean (task, TRUE); + g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_UNKNOWN, "oh no"); + } +} + +static gboolean +idle_cb (gpointer user_data) +{ + GTask *task = user_data; + return_twice (task); + g_object_unref (task); + + return G_SOURCE_REMOVE; +} + +static void +test_return_permutation (gboolean error_first, + gboolean return_in_idle) +{ + guint calls = 0; + GTask *task = NULL; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1525"); + + task = g_task_new (NULL, NULL, task_complete_cb, &calls); + g_task_set_task_data (task, GUINT_TO_POINTER (error_first), NULL); + + if (return_in_idle) + g_idle_add (idle_cb, g_object_ref (task)); + else + return_twice (task); + + while (calls == 0) + g_main_context_iteration (NULL, TRUE); + + g_assert_cmpint (calls, ==, 1); + + g_object_unref (task); +} + +/* Test that calling g_task_return_boolean() after g_task_return_error(), when + * returning in an idle callback, correctly results in a critical warning. */ +static void +test_return_in_idle_error_first (void) +{ + if (g_test_subprocess ()) + { + test_return_permutation (TRUE, TRUE); + return; + } + + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*assertion '!task->ever_returned' failed*"); +} + +/* Test that calling g_task_return_error() after g_task_return_boolean(), when + * returning in an idle callback, correctly results in a critical warning. */ +static void +test_return_in_idle_value_first (void) +{ + if (g_test_subprocess ()) + { + test_return_permutation (FALSE, TRUE); + return; + } + + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*assertion '!task->ever_returned' failed*"); +} + +/* Test that calling g_task_return_boolean() after g_task_return_error(), when + * returning synchronously, correctly results in a critical warning. */ +static void +test_return_error_first (void) +{ + if (g_test_subprocess ()) + { + test_return_permutation (TRUE, FALSE); + return; + } + + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*assertion '!task->ever_returned' failed*"); +} + +/* Test that calling g_task_return_error() after g_task_return_boolean(), when + * returning synchronously, correctly results in a critical warning. */ +static void +test_return_value_first (void) +{ + if (g_test_subprocess ()) + { + test_return_permutation (FALSE, FALSE); + return; + } + + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*assertion '!task->ever_returned' failed*"); +} int main (int argc, char **argv) @@ -1994,6 +2124,7 @@ main (int argc, char **argv) int ret; g_test_init (&argc, &argv, NULL); + g_test_bug_base (""); loop = g_main_loop_new (NULL, FALSE); main_thread = g_thread_self (); @@ -2021,6 +2152,10 @@ main (int argc, char **argv) g_test_add_func ("/gtask/return-pointer", test_return_pointer); g_test_add_func ("/gtask/object-keepalive", test_object_keepalive); g_test_add_func ("/gtask/legacy-error", test_legacy_error); + g_test_add_func ("/gtask/return/in-idle/error-first", test_return_in_idle_error_first); + 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); ret = g_test_run(); |