From 3b8bc8bacf1fe31cda44fb5293711e87989388ea Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Tue, 9 Sep 2014 13:58:38 -0400 Subject: [PATCH] GDesktopAppInfo: avoid inotify on missing dirs Some desktop file directories, like /usr/local/share/applications may be missing on some systems. When we try to inotify on these directories, this will result in a every-4-seconds poll being setup which is quite bad. This is an issue that should be fixed in inotify itself but the problem is much larger there. For now, we can work around it in GDesktopAppInfo by refusing to monitor missing directories. We may get some spurious notifications of changes in the case that /usr/local/share or /usr/local/share/applications is created without actually adding desktop files, but spurious changes can already be reported in other cases, so that's OK. We won't get (user-visible) notification for a simple case of a completely unrelated file being created (however we cannot avoid the wakeup in this case due to how inotify works). That's probably pretty theoretical, though, since files in /usr don't change much and for the home directory we're likely to have at least ~/.config and ~/.local existing. https://bugzilla.gnome.org/show_bug.cgi?id=736350 --- gio/gdesktopappinfo.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 3 deletions(-) diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index ddec690..4a9aac2 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -137,6 +137,7 @@ G_DEFINE_TYPE_WITH_CODE (GDesktopAppInfo, g_desktop_app_info, G_TYPE_OBJECT, typedef struct { gchar *path; + gchar *alternatively_watching; gboolean is_config; gboolean is_setup; GLocalDirectoryMonitor *monitor; @@ -155,6 +156,52 @@ static GMutex desktop_file_dir_lock; /* Monitor 'changed' signal handler {{{2 */ static void desktop_file_dir_reset (DesktopFileDir *dir); +/*< internal > + * desktop_file_dir_get_alternative_dir: + * @dir: a #DesktopFileDir + * + * Gets the "alternative" directory to monitor in case the path + * doesn't exist. + * + * If the path exists this will return NULL, otherwise it will return a + * parent directory of the path. + * + * This is used to avoid inotify on a non-existent directory (which + * results in polling). + * + * See https://bugzilla.gnome.org/show_bug.cgi?id=522314 for more info. + */ +static gchar * +desktop_file_dir_get_alternative_dir (DesktopFileDir *dir) +{ + gchar *parent; + + /* If the directory itself exists then we need no alternative. */ + if (g_access (dir->path, R_OK | X_OK) == 0) + return NULL; + + /* Otherwise, try the parent directories until we find one. */ + parent = g_path_get_dirname (dir->path); + + while (g_access (parent, R_OK | X_OK) != 0) + { + gchar *tmp = parent; + + parent = g_path_get_dirname (tmp); + + /* If somehow we get to '/' or '.' then just stop... */ + if (g_str_equal (parent, tmp)) + { + g_free (tmp); + break; + } + + g_free (tmp); + } + + return parent; +} + static void desktop_file_dir_changed (GFileMonitor *monitor, GFile *file, @@ -163,6 +210,7 @@ desktop_file_dir_changed (GFileMonitor *monitor, gpointer user_data) { DesktopFileDir *dir = user_data; + gboolean do_nothing = FALSE; /* We are not interested in receiving notifications forever just * because someone asked about one desktop file once. @@ -170,15 +218,30 @@ desktop_file_dir_changed (GFileMonitor *monitor, * After we receive the first notification, reset the dir, destroying * the monitor. We will take this as a hint, next time that we are * asked, that we need to check if everything is up to date. + * + * If this is a notification for a parent directory (because the + * desktop directory didn't exist) then we shouldn't fire the signal + * unless something actually changed. */ g_mutex_lock (&desktop_file_dir_lock); - desktop_file_dir_reset (dir); + if (dir->alternatively_watching) + { + gchar *alternative_dir; + + alternative_dir = desktop_file_dir_get_alternative_dir (dir); + do_nothing = alternative_dir && g_str_equal (dir->alternatively_watching, alternative_dir); + g_free (alternative_dir); + } + + if (!do_nothing) + desktop_file_dir_reset (dir); g_mutex_unlock (&desktop_file_dir_lock); /* Notify anyone else who may be interested */ - g_app_info_monitor_fire (); + if (!do_nothing) + g_app_info_monitor_fire (); } /* Internal utility functions {{{2 */ @@ -1207,6 +1270,12 @@ desktop_file_dir_create_for_config (GArray *array, static void desktop_file_dir_reset (DesktopFileDir *dir) { + if (dir->alternatively_watching) + { + g_free (dir->alternatively_watching); + dir->alternatively_watching = NULL; + } + if (dir->monitor) { g_signal_handlers_disconnect_by_func (dir->monitor, desktop_file_dir_changed, dir); @@ -1252,10 +1321,23 @@ desktop_file_dir_reset (DesktopFileDir *dir) static void desktop_file_dir_init (DesktopFileDir *dir) { + const gchar *watch_dir; + g_assert (!dir->is_setup); + g_assert (!dir->alternatively_watching); g_assert (!dir->monitor); - dir->monitor = g_local_directory_monitor_new_in_worker (dir->path, G_FILE_MONITOR_NONE, NULL); + + dir->alternatively_watching = desktop_file_dir_get_alternative_dir (dir); + watch_dir = dir->alternatively_watching ? dir->alternatively_watching : dir->path; + + /* There is a very thin race here if the watch_dir has been _removed_ + * between when we checked for it and when we establish the watch. + * Removes probably don't happen in usual operation, and even if it + * does (and we catch the unlikely race), the only degradation is that + * we will fall back to polling. + */ + dir->monitor = g_local_directory_monitor_new_in_worker (watch_dir, G_FILE_MONITOR_NONE, NULL); if (dir->monitor) { -- 2.7.4