From 218b298a1b8958e9313a460b674238d76191a3ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 30 Jan 2025 19:10:40 +0100 Subject: [PATCH 1/3] gio/gfilemonitor: Use atomic API to get / store cancelled state The cancelled state may be set and read by different threads, so ensure that it's stored and managed in an atomic way. We could in fact end up check for `g_file_monitor_is_cancelled()` in a thread and `g_file_monitor_cancel()` or `g_file_monitor_emit_event` in in another one. --- gio/gfilemonitor.c | 14 +++++--------- gio/tests/meson.build | 6 +++++- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gio/gfilemonitor.c b/gio/gfilemonitor.c index bcfa3636f..d89c54e17 100644 --- a/gio/gfilemonitor.c +++ b/gio/gfilemonitor.c @@ -25,6 +25,7 @@ #include "gfilemonitor.h" #include "gioenumtypes.h" +#include "glib.h" #include "gmarshal-internal.h" #include "gfile.h" #include "gvfs.h" @@ -52,7 +53,7 @@ struct _GFileMonitorPrivate { - gboolean cancelled; + int cancelled; /* atomic */ }; G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE (GFileMonitor, g_file_monitor, G_TYPE_OBJECT) @@ -219,13 +220,9 @@ g_file_monitor_class_init (GFileMonitorClass *klass) gboolean g_file_monitor_is_cancelled (GFileMonitor *monitor) { - gboolean res; - g_return_val_if_fail (G_IS_FILE_MONITOR (monitor), FALSE); - res = monitor->priv->cancelled; - - return res; + return g_atomic_int_get (&monitor->priv->cancelled); } /** @@ -241,11 +238,10 @@ g_file_monitor_cancel (GFileMonitor *monitor) { g_return_val_if_fail (G_IS_FILE_MONITOR (monitor), FALSE); - if (!monitor->priv->cancelled) + if (!g_atomic_int_exchange (&monitor->priv->cancelled, TRUE)) { G_FILE_MONITOR_GET_CLASS (monitor)->cancel (monitor); - monitor->priv->cancelled = TRUE; g_object_notify (G_OBJECT (monitor), "cancelled"); } @@ -293,7 +289,7 @@ g_file_monitor_emit_event (GFileMonitor *monitor, g_return_if_fail (G_IS_FILE (child)); g_return_if_fail (!other_file || G_IS_FILE (other_file)); - if (monitor->priv->cancelled) + if (g_atomic_int_get (&monitor->priv->cancelled)) return; g_signal_emit (monitor, g_file_monitor_changed_signal, 0, child, other_file, event_type); diff --git a/gio/tests/meson.build b/gio/tests/meson.build index 43c1bfe34..23fd5a208 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -134,7 +134,11 @@ gio_tests = { 'vfs' : {}, 'volumemonitor' : {}, 'glistmodel' : {}, - 'testfilemonitor' : {'suite' : ['slow', 'flaky']}, + 'testfilemonitor' : { + 'suite' : ['slow'], + # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/1392 + 'can_fail' : host_system in ['darwin'], + }, 'thumbnail-verification' : {}, 'tls-certificate' : {'extra_sources' : ['gtesttlsbackend.c']}, 'tls-interaction' : {'extra_sources' : ['gtesttlsbackend.c']}, From 175e02f15c817f1b0b3b0ac93868c70fe685b085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 31 Jan 2025 11:04:20 +0100 Subject: [PATCH 2/3] gio/gfilemonitor: Do not mark the filemonitor cancelled until vfunc is done As per previous commit we used atomic logic to handle the cancellation, but that lead to a slightly different behavior because the file monitor was then marked as cancelled before the vfunc implementation was called. Use similar behavior now (by still relying on the atomic logic), by marking the state as about-to-cancel as soon as we're starting the cancellation (preventing other threads to cancel it), and eventually fully marking it as cancelled. --- gio/gfilemonitor.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/gio/gfilemonitor.c b/gio/gfilemonitor.c index d89c54e17..a769c2cf5 100644 --- a/gio/gfilemonitor.c +++ b/gio/gfilemonitor.c @@ -51,6 +51,12 @@ #define DEFAULT_RATE_LIMIT_MSECS 800 +typedef enum { + CANCEL_STATE_NONE, + CANCEL_STATE_CANCELLING, + CANCEL_STATE_CANCELLED, +} GFileMonitorCancelState; + struct _GFileMonitorPrivate { int cancelled; /* atomic */ @@ -222,7 +228,7 @@ g_file_monitor_is_cancelled (GFileMonitor *monitor) { g_return_val_if_fail (G_IS_FILE_MONITOR (monitor), FALSE); - return g_atomic_int_get (&monitor->priv->cancelled); + return g_atomic_int_get (&monitor->priv->cancelled) == CANCEL_STATE_CANCELLED; } /** @@ -238,10 +244,13 @@ g_file_monitor_cancel (GFileMonitor *monitor) { g_return_val_if_fail (G_IS_FILE_MONITOR (monitor), FALSE); - if (!g_atomic_int_exchange (&monitor->priv->cancelled, TRUE)) + if (g_atomic_int_compare_and_exchange (&monitor->priv->cancelled, + CANCEL_STATE_NONE, + CANCEL_STATE_CANCELLING)) { G_FILE_MONITOR_GET_CLASS (monitor)->cancel (monitor); + g_atomic_int_set (&monitor->priv->cancelled, CANCEL_STATE_CANCELLED); g_object_notify (G_OBJECT (monitor), "cancelled"); } @@ -289,7 +298,7 @@ g_file_monitor_emit_event (GFileMonitor *monitor, g_return_if_fail (G_IS_FILE (child)); g_return_if_fail (!other_file || G_IS_FILE (other_file)); - if (g_atomic_int_get (&monitor->priv->cancelled)) + if (g_atomic_int_get (&monitor->priv->cancelled) != CANCEL_STATE_NONE) return; g_signal_emit (monitor, g_file_monitor_changed_signal, 0, child, other_file, event_type); From 795a56d62343d89b2213428b25e8f65c6bd9cceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 31 Jan 2025 11:27:55 +0100 Subject: [PATCH 3/3] gio/gfilemonitor: Reorder includes as expected by syntax checker --- gio/gfilemonitor.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gio/gfilemonitor.c b/gio/gfilemonitor.c index a769c2cf5..f78fad2e9 100644 --- a/gio/gfilemonitor.c +++ b/gio/gfilemonitor.c @@ -23,13 +23,13 @@ #include "config.h" #include +#include "gfile.h" #include "gfilemonitor.h" #include "gioenumtypes.h" #include "glib.h" -#include "gmarshal-internal.h" -#include "gfile.h" -#include "gvfs.h" #include "glibintl.h" +#include "gmarshal-internal.h" +#include "gvfs.h" /** * GFileMonitor: