luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-04-15 12:08:04 +02:00
Code Issues Packages Projects Releases Wiki Activity

gio/cancellable: Never call toggle notify callback in locked state

Browse Source 
We had code to avoid that we could call a toggle "up" notification
callback in locked state, but this was not covering the case in which
the cancellable second to last reference was removed in its cancellation
callback.

In fact, in such case we end up going from 2 -> 1 references during the
signal callback call and this leads to calling the toggle notify
callback in locked state.

To prevent this, add an even further reference before calling the
callback (in locked state, but there's no risk that a toggle-up
notification happens now), and drop it once unlocked again.
This commit is contained in:
Marco Trevisan (Treviño) 2025-04-02 20:36:21 +02:00
parent ec2fee555f
commit a2f98c86ff
2 changed files with 172 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
gio/gcancellable.c
View File

@ -531,6 +531,13 @@ g_cancellable_cancel (GCancellable *cancellable)
if (priv->wakeup)
GLIB_PRIVATE_CALL (g_wakeup_signal) (priv->wakeup);
/* Adding another reference, in case the callback is unreffing the
* cancellable and there are toggle references, so that the second to last
* reference (that would lead a toggle notification) won't be released
* while we're locked.
*/
g_object_ref (cancellable);
g_signal_emit (cancellable, signals[CANCELLED], 0);
if (g_atomic_int_dec_and_test (&priv->cancelled_running))
@ -539,6 +546,7 @@ g_cancellable_cancel (GCancellable *cancellable)
g_mutex_unlock (&priv->mutex);
g_object_unref (cancellable);
g_object_unref (cancellable);
}
/**
@ -595,6 +603,7 @@ g_cancellable_connect (GCancellable *cancellable,
gpointer data,
GDestroyNotify data_destroy_func)
{
GCancellable *extra_ref = NULL;
gulong id;
g_return_val_if_fail (G_IS_CANCELLABLE (cancellable), 0);
@ -616,6 +625,13 @@ g_cancellable_connect (GCancellable *cancellable,
void (*_callback) (GCancellable *cancellable,
gpointer user_data);
/* Adding another reference, in case the callback is unreffing the
* cancellable and there are toggle references, so that the second to last
* reference (that would lead a toggle notification) won't be released
* while we're locked.
*/
extra_ref = g_object_ref (cancellable);
_callback = (void *)callback;
id = 0;
@ -638,6 +654,7 @@ g_cancellable_connect (GCancellable *cancellable,
data_destroy_func (data);
g_object_unref (cancellable);
g_clear_object (&extra_ref);
return id;
}

158
gio/tests/cancellable.c
View File

@ -952,12 +952,162 @@ test_connect_to_disposing_callback (void)
g_object_add_weak_pointer (G_OBJECT (cancellable), (gpointer *) &cancellable);
id = g_cancellable_connect (cancellable, G_CALLBACK (assert_references_and_unref),
GINT_TO_POINTER (3), NULL);
GINT_TO_POINTER (4), NULL);
g_assert_cmpuint (id, >, 0);
g_cancellable_cancel (cancellable);
g_assert_null (cancellable);
}
typedef struct
{
gulong id;
gboolean ignore_next_toggle_down;
} ToggleReferenceData;
static void
toggle_reference_cb (gpointer data,
GObject *object,
gboolean is_last_ref)
{
GCancellable *cancellable = G_CANCELLABLE (object);
ToggleReferenceData *toggle_data = data;
g_test_message ("Toggle reference callback for %s (%p), last: %d",
g_type_name_from_instance ((GTypeInstance *) object),
object, is_last_ref);
if (!is_last_ref)
{
g_assert_false (g_cancellable_is_cancelled (cancellable));
/* Disconnect and reconnect to the signal so that we can verify that the
* "toggle-up" does not happen while we're locked
*/
g_cancellable_disconnect (cancellable, toggle_data->id);
toggle_data->id = g_cancellable_connect (cancellable,
G_CALLBACK (assert_references_and_unref),
GINT_TO_POINTER (4), NULL);
return;
}
g_assert_true (is_last_ref);
if (toggle_data->ignore_next_toggle_down)
{
g_assert_false (g_cancellable_is_cancelled (cancellable));
toggle_data->ignore_next_toggle_down = FALSE;
return;
}
g_assert_true (g_cancellable_is_cancelled (cancellable));
/* This would deadlock if the last reference was removed during cancellation */
g_cancellable_disconnect (cancellable, toggle_data->id);
toggle_data->id = 0;
}
static void
test_connect_to_disposing_callback_with_toggle_reference (void)
{
GCancellable *cancellable;
ToggleReferenceData data = {0};
cancellable = g_cancellable_new ();
g_object_add_weak_pointer (G_OBJECT (cancellable), (gpointer *) &cancellable);
data.id = g_cancellable_connect (cancellable, G_CALLBACK (assert_references_and_unref),
GINT_TO_POINTER (4), NULL);
/* Switch to toggle references. */
g_object_add_toggle_ref (G_OBJECT (cancellable), toggle_reference_cb, &data);
data.ignore_next_toggle_down = TRUE;
g_object_unref (cancellable);
g_cancellable_cancel (cancellable);
g_assert_cmpuint (data.id, ==, 0);
g_assert_null (cancellable);
}
static void
cancelled_toggle_reference_cb (gpointer data,
GObject *object,
gboolean is_last_ref)
{
GCancellable *cancellable = G_CANCELLABLE (object);
ToggleReferenceData *toggle_data = data;
g_test_message ("Toggle reference callback for %s (%p), last: %d",
g_type_name_from_instance ((GTypeInstance *) object),
object, is_last_ref);
if (!is_last_ref)
{
gulong id;
if (g_cancellable_is_cancelled (cancellable))
{
/* Disconnect and reconnect to the signal so that we can verify that the
* "toggle-up" does not happen while we're locked
*/
g_cancellable_disconnect (cancellable, toggle_data->id);
id = g_cancellable_connect (cancellable, G_CALLBACK (do_nothing), NULL, NULL);
g_assert_cmpuint (id, ==, 0);
return;
}
/* Connect and disconnect to the signal so that we can verify that the
* "toggle-up" does not happen while we're locked
*/
id = g_cancellable_connect (cancellable, G_CALLBACK (do_nothing), NULL, NULL);
g_cancellable_disconnect (cancellable, id);
return;
}
g_assert_true (is_last_ref);
if (toggle_data->ignore_next_toggle_down)
{
g_assert_false (g_cancellable_is_cancelled (cancellable));
toggle_data->ignore_next_toggle_down = FALSE;
return;
}
g_assert_true (g_cancellable_is_cancelled (cancellable));
g_test_expect_message ("GLib-GObject", G_LOG_LEVEL_CRITICAL,
"*has no handler with id*");
/* We try resetting the a signal that isn't connected, since we don't care
* about anything but checking wether this would deadlock
*/
g_cancellable_disconnect (cancellable, G_MAXULONG);
g_test_assert_expected_messages ();
}
static void
test_connect_cancelled_to_disposing_callback_with_toggle_reference (void)
{
GCancellable *cancellable;
ToggleReferenceData data = {0};
gulong id;
cancellable = g_cancellable_new ();
g_object_add_weak_pointer (G_OBJECT (cancellable), (gpointer *) &cancellable);
/* Switch to toggle references. */
g_object_add_toggle_ref (G_OBJECT (cancellable), cancelled_toggle_reference_cb, &data);
data.ignore_next_toggle_down = TRUE;
g_object_unref (cancellable);
g_cancellable_cancel (cancellable);
id = g_cancellable_connect (cancellable, G_CALLBACK (assert_references_and_unref),
GINT_TO_POINTER (3), NULL);
g_assert_cmpuint (id, ==, 0);
g_assert_null (cancellable);
}
static void
test_connect_cancelled_to_disposing_callback (void)
{
@ -972,7 +1122,7 @@ test_connect_cancelled_to_disposing_callback (void)
g_cancellable_cancel (cancellable);
id = g_cancellable_connect (cancellable, G_CALLBACK (assert_references_and_unref),
GINT_TO_POINTER (2), NULL);
GINT_TO_POINTER (3), NULL);
g_assert_cmpuint (id, ==, 0);
g_assert_null (cancellable);
}
@ -1005,7 +1155,7 @@ test_connect_cancelled_with_destroy_func_disposing_cancellable (void)
gulong id;
cancellable = g_cancellable_new ();
g_object_add_weak_pointer (G_OBJECT (cancellable), (gpointer*) &cancellable);
g_object_add_weak_pointer (G_OBJECT (cancellable), (gpointer *) &cancellable);
g_cancellable_cancel (cancellable);
id = g_cancellable_connect (cancellable, G_CALLBACK (on_connect_data_callback),
@ -1025,8 +1175,10 @@ main (int argc, char *argv[])
g_test_add_func ("/cancellable/connect-data-is-destroyed-on-disconnect-and-dispose", test_connect_data_is_destroyed_on_disconnect_and_dispose);
g_test_add_func ("/cancellable/connect-to-disposing-callback", test_connect_to_disposing_callback);
g_test_add_func ("/cancellable/connect-cancelled-data-is-destroyed", test_connect_cancelled_data_is_destroyed);
g_test_add_func ("/cancellable/connect-to-disposing-callback-with-toggle-reference", test_connect_to_disposing_callback_with_toggle_reference);
g_test_add_func ("/cancellable/connect-cancelled-to-disposing-callback", test_connect_cancelled_to_disposing_callback);
g_test_add_func ("/cancellable/connect-cancelled-with-destroy-func-disposing-cancellable", test_connect_cancelled_with_destroy_func_disposing_cancellable);
g_test_add_func ("/cancellable/connect-cancelled-to-disposing-callback-with-toggle-reference", test_connect_cancelled_to_disposing_callback_with_toggle_reference);
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);