From 8e2854edd33cace8e37167a97b640110d17e9e46 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Thu, 1 Feb 2024 18:13:55 -0600 Subject: [PATCH 1/5] Increase availibility of g_unix_mount_get_options() --- gio/gunixmounts.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/gunixmounts.h b/gio/gunixmounts.h index 72b210dc1..6bef2a323 100644 --- a/gio/gunixmounts.h +++ b/gio/gunixmounts.h @@ -81,7 +81,7 @@ GLIB_AVAILABLE_IN_ALL const char * g_unix_mount_get_device_path (GUnixMountEntry *mount_entry); GLIB_AVAILABLE_IN_ALL const char * g_unix_mount_get_fs_type (GUnixMountEntry *mount_entry); -GLIB_AVAILABLE_IN_2_58 +GLIB_AVAILABLE_IN_2_56 const char * g_unix_mount_get_options (GUnixMountEntry *mount_entry); GLIB_AVAILABLE_IN_ALL gboolean g_unix_mount_is_readonly (GUnixMountEntry *mount_entry); -- 2.43.0 From b2bdd8ce2a1af5d5516d9df9564cfd94545ffbfb Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Mon, 10 Feb 2020 16:00:42 +0100 Subject: [PATCH 2/5] gunixmounts: Make GUnixMountMonitor thread-safe The Nautilus test suite often crashes with "GLib-FATAL-CRITICAL: g_source_is_destroyed: assertion 'g_atomic_int_get (&source->ref_count) > 0' failed" if it is started with "GIO_USE_VOLUME_MONITOR=unix". This is because GUnixMountMonitor is simultaneously used from multiple threads over GLocalFile and GVolumeMonitor APIs. Let's add guards for proc_mounts_watch_source and mount_poller_time variables to prevent those crashes. Fixes: https://gitlab.gnome.org/GNOME/glib/issues/2030 --- gio/gunixmounts.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/gio/gunixmounts.c b/gio/gunixmounts.c index 3514e2e8e..a0cd388c4 100644 --- a/gio/gunixmounts.c +++ b/gio/gunixmounts.c @@ -151,7 +151,11 @@ static GList *_g_get_unix_mounts (void); static GList *_g_get_unix_mount_points (void); static gboolean proc_mounts_watch_is_running (void); +G_LOCK_DEFINE_STATIC (proc_mounts_source); + +/* Protected by proc_mounts_source lock */ static guint64 mount_poller_time = 0; +static GSource *proc_mounts_watch_source; #ifdef HAVE_SYS_MNTTAB_H #define MNTOPT_RO "ro" @@ -1450,18 +1454,21 @@ get_mounts_timestamp (void) { const char *monitor_file; struct stat buf; + guint64 timestamp = 0; + + G_LOCK (proc_mounts_source); monitor_file = get_mtab_monitor_file (); /* Don't return mtime for /proc/ files */ if (monitor_file && !g_str_has_prefix (monitor_file, "/proc/")) { if (stat (monitor_file, &buf) == 0) - return (guint64)buf.st_mtime; + timestamp = buf.st_mtime; } else if (proc_mounts_watch_is_running ()) { /* it's being monitored by poll, so return mount_poller_time */ - return mount_poller_time; + timestamp = mount_poller_time; } else { @@ -1470,9 +1477,12 @@ get_mounts_timestamp (void) * return TRUE so any application caches depending on it (like eg. * the one in GIO) get invalidated and don't hold possibly outdated * data - see Bug 787731 */ - return (guint64) g_get_monotonic_time (); + timestamp = g_get_monotonic_time (); } - return 0; + + G_UNLOCK (proc_mounts_source); + + return timestamp; } static guint64 @@ -1704,10 +1714,10 @@ G_DEFINE_TYPE (GUnixMountMonitor, g_unix_mount_monitor, G_TYPE_OBJECT) static GContextSpecificGroup mount_monitor_group; static GFileMonitor *fstab_monitor; static GFileMonitor *mtab_monitor; -static GSource *proc_mounts_watch_source; static GList *mount_poller_mounts; static guint mtab_file_changed_id; +/* Called with proc_mounts_source lock held. */ static gboolean proc_mounts_watch_is_running (void) { @@ -1768,7 +1778,10 @@ proc_mounts_changed (GIOChannel *channel, { if (cond & G_IO_ERR) { + G_LOCK (proc_mounts_source); mount_poller_time = (guint64) g_get_monotonic_time (); + G_UNLOCK (proc_mounts_source); + g_context_specific_group_emit (&mount_monitor_group, signals[MOUNTS_CHANGED]); } @@ -1802,7 +1815,10 @@ mount_change_poller (gpointer user_data) if (has_changed) { + G_LOCK (proc_mounts_source); mount_poller_time = (guint64) g_get_monotonic_time (); + G_UNLOCK (proc_mounts_source); + g_context_specific_group_emit (&mount_monitor_group, signals[MOUNTPOINTS_CHANGED]); } @@ -1819,11 +1835,13 @@ mount_monitor_stop (void) g_object_unref (fstab_monitor); } + G_LOCK (proc_mounts_source); if (proc_mounts_watch_source != NULL) { g_source_destroy (proc_mounts_watch_source); proc_mounts_watch_source = NULL; } + G_UNLOCK (proc_mounts_source); if (mtab_monitor) { @@ -1869,6 +1887,8 @@ mount_monitor_start (void) } else { + G_LOCK (proc_mounts_source); + proc_mounts_watch_source = g_io_create_watch (proc_mounts_channel, G_IO_ERR); g_source_set_callback (proc_mounts_watch_source, (GSourceFunc) proc_mounts_changed, @@ -1877,6 +1897,8 @@ mount_monitor_start (void) g_main_context_get_thread_default ()); g_source_unref (proc_mounts_watch_source); g_io_channel_unref (proc_mounts_channel); + + G_UNLOCK (proc_mounts_source); } } else @@ -1889,6 +1911,8 @@ mount_monitor_start (void) } else { + G_LOCK (proc_mounts_source); + proc_mounts_watch_source = g_timeout_source_new_seconds (3); mount_poller_mounts = _g_get_unix_mounts (); mount_poller_time = (guint64)g_get_monotonic_time (); @@ -1898,6 +1922,8 @@ mount_monitor_start (void) g_source_attach (proc_mounts_watch_source, g_main_context_get_thread_default ()); g_source_unref (proc_mounts_watch_source); + + G_UNLOCK (proc_mounts_source); } } -- 2.43.0 From 98e7cc97852d148b9ed5276c58470ba59823dd44 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Tue, 18 Feb 2020 09:10:03 +0100 Subject: [PATCH 3/5] gunixmounts: Prevent invalid time_read timestamps The `get_mounts_timestamp()` function uses `mount_poller_time` when `proc_mounts_watch_source` is set, but the `mount_poller_time` is not initialized in the same time as `proc_mounts_watch_source`. This may cause that zero, or some outdated value is returned. Let's initialize `mount_poller_time` to prevent invalid values to be returned. --- gio/gunixmounts.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gio/gunixmounts.c b/gio/gunixmounts.c index a0cd388c4..62b256f77 100644 --- a/gio/gunixmounts.c +++ b/gio/gunixmounts.c @@ -1890,6 +1890,7 @@ mount_monitor_start (void) G_LOCK (proc_mounts_source); proc_mounts_watch_source = g_io_create_watch (proc_mounts_channel, G_IO_ERR); + mount_poller_time = (guint64) g_get_monotonic_time (); g_source_set_callback (proc_mounts_watch_source, (GSourceFunc) proc_mounts_changed, NULL, NULL); -- 2.43.0 From e1b9019e3206b9a2dc74c7e587c6367cf33047da Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Mon, 17 Feb 2020 11:05:07 +0100 Subject: [PATCH 4/5] gunixmounts: Prevent race when mtab file changed mtab_file_changed_id might be set on thread default context, but it is always cleared on the global context because of usage of g_idle_add. This can cause the emission of redundant "mounts-change" signals. This should not cause any issues to the client application, but let's attach the idle source to the thread-default context instead to avoid those races for sure. --- gio/gunixmounts.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/gio/gunixmounts.c b/gio/gunixmounts.c index 62b256f77..d01b53c8d 100644 --- a/gio/gunixmounts.c +++ b/gio/gunixmounts.c @@ -1756,6 +1756,9 @@ mtab_file_changed (GFileMonitor *monitor, GFileMonitorEvent event_type, gpointer user_data) { + GMainContext *context; + GSource *source; + if (event_type != G_FILE_MONITOR_EVENT_CHANGED && event_type != G_FILE_MONITOR_EVENT_CREATED && event_type != G_FILE_MONITOR_EVENT_DELETED) @@ -1768,7 +1771,16 @@ mtab_file_changed (GFileMonitor *monitor, if (mtab_file_changed_id > 0) return; - mtab_file_changed_id = g_idle_add (mtab_file_changed_cb, NULL); + context = g_main_context_get_thread_default (); + if (!context) + context = g_main_context_default (); + + source = g_idle_source_new (); + g_source_set_priority (source, G_PRIORITY_DEFAULT); + g_source_set_callback (source, mtab_file_changed_cb, NULL, NULL); + g_source_set_name (source, "[gio] mtab_file_changed_cb"); + g_source_attach (source, context); + g_source_unref (source); } static gboolean -- 2.43.0 From 7dc55e61bd174561851dfaa50fa1420428fb7714 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Mon, 17 Feb 2020 11:20:17 +0100 Subject: [PATCH 5/5] gunixmounts: Remove pending sources when finalizing mtab_file_changed_id is not currently removed when finalizing, which could potentially lead to segfaults. Let's remove the source when finalizing to avoid this. --- gio/gunixmounts.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gio/gunixmounts.c b/gio/gunixmounts.c index d01b53c8d..4695d5618 100644 --- a/gio/gunixmounts.c +++ b/gio/gunixmounts.c @@ -1861,6 +1861,12 @@ mount_monitor_stop (void) g_object_unref (mtab_monitor); } + if (mtab_file_changed_id) + { + g_source_remove (mtab_file_changed_id); + mtab_file_changed_id = 0; + } + g_list_free_full (mount_poller_mounts, (GDestroyNotify) g_unix_mount_free); } -- 2.43.0