gio/cancellable: Always add reference while emitting cancelled signal

When a non-cancelled cancellable ::cancelled signal callback is called
the cancellable has enough references so that it can be unreferenced on
the callback itself. However this doesn't happen if the cancellable has
been already cancelled at the moment we connect to it.

To prevent this, add a temporary reference before calling the signal
callback.

Note that we do this also if the callback has not been already cancelled
to prevent that we may end up calling a toggle-notify callback while we
are locked.

Add tests

Closes: #3643
This commit is contained in:
Marco Trevisan (Treviño)
2025-04-02 19:18:27 +02:00
parent 7432441581
commit 49680afd5c
2 changed files with 32 additions and 0 deletions

View File

@@ -566,6 +566,16 @@ g_cancellable_connect (GCancellable *cancellable,
g_return_val_if_fail (G_IS_CANCELLABLE (cancellable), 0);
/* If the cancellable is already cancelled we may end up calling the callback
* immediately, and the callback may unref the Cancellable, so we need to add
* an extra reference here. We can't do it only in the case the cancellable
* is already cancelled because it can be potentially be reset, so we can't
* rely on the atomic value only, but we need to be locked to be really sure.
* At the same time we don't want to wake up the ToggleNotify if toggle
* references are enabled while we're locked.
*/
g_object_ref (cancellable);
g_mutex_lock (&cancellable->priv->mutex);
if (g_atomic_int_get (&cancellable->priv->cancelled))
@@ -593,6 +603,8 @@ g_cancellable_connect (GCancellable *cancellable,
g_mutex_unlock (&cancellable->priv->mutex);
g_object_unref (cancellable);
return id;
}

View File

@@ -909,6 +909,25 @@ test_connect_to_disposing_callback (void)
g_assert_null (cancellable);
}
static void
test_connect_cancelled_to_disposing_callback (void)
{
GCancellable *cancellable;
gulong id;
g_test_summary ("A cancellable signal callback can unref the cancellable");
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3643");
cancellable = g_cancellable_new ();
g_object_add_weak_pointer (G_OBJECT (cancellable), (gpointer *) &cancellable);
g_cancellable_cancel (cancellable);
id = g_cancellable_connect (cancellable, G_CALLBACK (assert_references_and_unref),
GINT_TO_POINTER (2), NULL);
g_assert_cmpuint (id, ==, 0);
g_assert_null (cancellable);
}
int
main (int argc, char *argv[])
{
@@ -917,6 +936,7 @@ main (int argc, char *argv[])
g_test_add_func ("/cancellable/multiple-concurrent", test_cancel_multiple_concurrent);
g_test_add_func ("/cancellable/null", test_cancel_null);
g_test_add_func ("/cancellable/connect-to-disposing-callback", test_connect_to_disposing_callback);
g_test_add_func ("/cancellable/connect-cancelled-to-disposing-callback", test_connect_cancelled_to_disposing_callback);
g_test_add_func ("/cancellable/disconnect-on-cancelled-callback-hangs", test_cancellable_disconnect_on_cancelled_callback_hangs);
g_test_add_func ("/cancellable/resets-on-cancel-callback-hangs", test_cancellable_reset_on_cancelled_callback_hangs);
g_test_add_func ("/cancellable/poll-fd", test_cancellable_poll_fd);