ibus-daemon: use GFileMonitor instead of polling
authorDaiki Ueno <ueno@unixuser.org>
Fri, 26 Oct 2012 07:56:34 +0000 (16:56 +0900)
committerDaiki Ueno <ueno@unixuser.org>
Fri, 26 Oct 2012 07:56:34 +0000 (16:56 +0900)
BUG=Issue#1514

Review URL: https://codereview.appspot.com/6589065

bus/global.c
bus/global.h
bus/ibusimpl.c
bus/main.c
bus/registry.c
bus/registry.h
bus/server.c
src/ibuscomponent.c
src/ibuscomponent.h
src/ibusobservedpath.c
src/ibusobservedpath.h

index 47b8721..c9dd31b 100644 (file)
@@ -29,7 +29,3 @@ gchar *g_cache = "auto";
 gboolean g_mempro = FALSE;
 gboolean g_verbose = FALSE;
 gint   g_gdbus_timeout = 5000;
-#ifdef G_THREADS_ENABLED
-gint   g_monitor_timeout = 0;
-#endif
-
index 7bf2a1a..db7e992 100644 (file)
@@ -34,9 +34,6 @@ extern gchar *g_cache;
 extern gboolean g_mempro;
 extern gboolean g_verbose;
 extern gint   g_gdbus_timeout;
-#ifdef G_THREADS_ENABLED
-extern gint   g_monitor_timeout;
-#endif
 
 G_END_DECLS
 
index 25d2523..cafb34a 100644 (file)
@@ -330,13 +330,7 @@ bus_ibus_impl_init (BusIBusImpl *ibus)
                       "changed",
                       G_CALLBACK (_registry_changed_cb),
                       ibus);
-#ifdef G_THREADS_ENABLED
-    extern gint g_monitor_timeout;
-    if (g_monitor_timeout != 0) {
-        /* Start the monitor of registry changes. */
-        bus_registry_start_monitor_changes (ibus->registry);
-    }
-#endif
+    bus_registry_start_monitor_changes (ibus->registry);
 
     ibus->keymap = ibus_keymap_get ("us");
 
index d59b5b7..791a524 100644 (file)
@@ -65,9 +65,6 @@ static const GOptionEntry entries[] =
     { "replace",   'r', 0, G_OPTION_ARG_NONE,   &replace,   "if there is an old ibus-daemon is running, it will be replaced.", NULL },
     { "cache",     't', 0, G_OPTION_ARG_STRING, &g_cache,   "specify the cache mode. [auto/refresh/none]", NULL },
     { "timeout",   'o', 0, G_OPTION_ARG_INT,    &g_gdbus_timeout, "gdbus reply timeout in milliseconds. pass -1 to use the default timeout of gdbus.", "timeout [default is 5000]" },
-#ifdef G_THREADS_ENABLED
-    { "monitor-timeout", 'j', 0, G_OPTION_ARG_INT,    &g_monitor_timeout, "timeout of poll changes of engines in seconds. 0 to disable it. ", "timeout [default is 0]" },
-#endif
     { "mem-profile", 'm', 0, G_OPTION_ARG_NONE,   &g_mempro,   "enable memory profile, send SIGUSR2 to print out the memory profile.", NULL },
     { "restart",     'R', 0, G_OPTION_ARG_NONE,   &restart,    "restart panel and config processes when they die.", NULL },
     { "verbose",   'v', 0, G_OPTION_ARG_NONE,   &g_verbose,   "verbose.", NULL },
index 5b0649a..9f917e7 100644 (file)
@@ -49,14 +49,10 @@ struct _BusRegistry {
     GList *components;
     /* a mapping from an engine name (e.g. 'pinyin') to the corresponding IBusEngineDesc object. */
     GHashTable *engine_table;
-
-#ifdef G_THREADS_ENABLED
-    GThread *thread;
-    gboolean thread_running;
-    GMutex  *mutex;
-    GCond   *cond;
     gboolean changed;
-#endif
+    /* a mapping from GFile to GFileMonitor. */
+    GHashTable *monitor_table;
+    guint monitor_timeout_id;
 };
 
 struct _BusRegistryClass {
@@ -103,22 +99,12 @@ bus_registry_init (BusRegistry *registry)
     registry->observed_paths = NULL;
     registry->components = NULL;
     registry->engine_table = g_hash_table_new (g_str_hash, g_str_equal);
-
-#ifdef G_THREADS_ENABLED
-    /* If glib supports thread, we'll create a thread to monitor changes in IME
-     * XML files and related files, so users can use newly installed IMEs
-     * immediatlly.
-     * Note that we don't use GFileMonitor for watching as we need to monitor
-     * not only XML files but also other related files that can be scattered in
-     * many places. Monitoring these files with GFileMonitor would be make it
-     * complicated. hence we use a thread to poll the changes.
-     */
-    registry->thread = NULL;
-    registry->thread_running = TRUE;
-    registry->mutex = g_mutex_new ();
-    registry->cond = g_cond_new ();
     registry->changed = FALSE;
-#endif
+    registry->monitor_table =
+        g_hash_table_new_full (g_file_hash,
+                               (GEqualFunc) g_file_equal,
+                               (GDestroyNotify) g_object_unref,
+                               (GDestroyNotify) g_object_unref);
 
     if (g_strcmp0 (g_cache, "none") == 0) {
         /* Only load registry, but not read and write cache. */
@@ -171,40 +157,24 @@ bus_registry_remove_all (BusRegistry *registry)
     registry->components = NULL;
 
     g_hash_table_remove_all (registry->engine_table);
+    g_hash_table_remove_all (registry->monitor_table);
 }
 
 static void
 bus_registry_destroy (BusRegistry *registry)
 {
-#ifdef G_THREADS_ENABLED
-    if (registry->thread) {
-        g_mutex_lock (registry->mutex);
-        registry->thread_running = FALSE;
-        g_mutex_unlock (registry->mutex);
-
-        /* Raise a signal to cause the loop in _checks_changes() exits
-         * immediately, and then wait until the thread finishes, and release all
-         * resources of the thread.
-         */
-        g_cond_signal (registry->cond);
-        g_thread_join (registry->thread);
-
-        registry->thread = NULL;
-    }
-#endif
-
     bus_registry_remove_all (registry);
 
     g_hash_table_destroy (registry->engine_table);
     registry->engine_table = NULL;
 
-#ifdef G_THREADS_ENABLED
-    g_cond_free (registry->cond);
-    registry->cond = NULL;
+    g_hash_table_destroy (registry->monitor_table);
+    registry->monitor_table = NULL;
 
-    g_mutex_free (registry->mutex);
-    registry->mutex = NULL;
-#endif
+    if (registry->monitor_timeout_id > 0) {
+        g_source_remove (registry->monitor_timeout_id);
+        registry->monitor_timeout_id = 0;
+    }
 
     IBUS_OBJECT_CLASS (bus_registry_parent_class)->destroy (IBUS_OBJECT (registry));
 }
@@ -554,56 +524,42 @@ bus_registry_stop_all_components (BusRegistry *registry)
 
 }
 
-#ifdef G_THREADS_ENABLED
 static gboolean
-_emit_changed_signal_cb (BusRegistry *registry)
+_monitor_timeout_cb (BusRegistry *registry)
 {
-    g_assert (BUS_IS_REGISTRY (registry));
-
+    g_hash_table_remove_all (registry->monitor_table);
+    registry->changed = TRUE;
     g_signal_emit (registry, _signals[CHANGED], 0);
+    registry->monitor_timeout_id = 0;
     return FALSE;
 }
 
-static gpointer
-_check_changes (BusRegistry *registry)
+static void
+_monitor_changed_cb (GFileMonitor     *monitor,
+                     GFile            *file,
+                     GFile            *other_file,
+                     GFileMonitorEvent event_type,
+                     BusRegistry      *registry)
 {
     g_assert (BUS_IS_REGISTRY (registry));
 
-    g_mutex_lock (registry->mutex);
-    while (registry->thread_running == TRUE && registry->changed == FALSE) {
-        extern gint g_monitor_timeout;
-        GTimeVal tv;
-        g_get_current_time (&tv);
-        g_time_val_add (&tv, g_monitor_timeout * G_USEC_PER_SEC);
-        /* Wait for the condition change or timeout. It will also unlock
-         * the mutex, so that other thread could obay the lock and modify
-         * the condition value.
-         * Note that we use g_cond_timed_wait() here rather than sleep() so
-         * that the loop can terminate immediately when the registry object
-         * is destroyed. See also comments in bus_registry_destroy().
-         */
-        if (g_cond_timed_wait (registry->cond, registry->mutex, &tv) == FALSE) {
-            /* Timeout happens. We check the modification of all IMEs' xml files
-             * and related files specified in <observed-paths> in xml files.
-             * If any file is changed, the changed signal will be emitted in
-             * main thread. It is for finding install/uninstall/upgrade of IMEs.
-             * On Linux desktop, ibus will popup UI to notificate users that
-             * some IMEs are changed and ibus need a restart.
-             */
-            if (bus_registry_check_modification (registry)) {
-                /* Emit the changed signal in main thread, and terminate
-                 * this thread.
-                 */
-                registry->changed = TRUE;
-                g_idle_add ((GSourceFunc) _emit_changed_signal_cb, registry);
-                break;
-            }
-        }
-        else
-            g_warn_if_fail (registry->thread_running == FALSE);
-    }
-    g_mutex_unlock (registry->mutex);
-    return NULL;
+    if (event_type != G_FILE_MONITOR_EVENT_CHANGED &&
+        event_type != G_FILE_MONITOR_EVENT_DELETED &&
+        event_type != G_FILE_MONITOR_EVENT_CREATED &&
+        event_type != G_FILE_MONITOR_EVENT_ATTRIBUTE_CHANGED)
+        return;
+
+    /* Merge successive file changes into one, with a low priority
+       timeout handler. */
+    if (registry->monitor_timeout_id > 0)
+        return;
+
+    registry->monitor_timeout_id =
+        g_timeout_add_full (G_PRIORITY_DEFAULT_IDLE,
+                            5000,
+                            (GSourceFunc) _monitor_timeout_cb,
+                            g_object_ref (registry),
+                            (GDestroyNotify) g_object_unref);
 }
 
 /**
@@ -614,16 +570,53 @@ _check_changes (BusRegistry *registry)
 void
 bus_registry_start_monitor_changes (BusRegistry *registry)
 {
+    GList *observed_paths, *p;
+
     g_assert (BUS_IS_REGISTRY (registry));
 
-    g_return_if_fail (registry->thread == NULL);
-    g_return_if_fail (registry->changed == FALSE);
+    g_hash_table_remove_all (registry->monitor_table);
 
-    registry->thread_running = TRUE;
-    registry->thread = g_thread_create ((GThreadFunc) _check_changes,
-                                        registry,
-                                        TRUE,
-                                        NULL);
+    observed_paths = g_list_copy (registry->observed_paths);
+    for (p = registry->components; p != NULL; p = p->next) {
+        BusComponent *buscomp = (BusComponent *) p->data;
+        IBusComponent *component = bus_component_get_component (buscomp);
+        GList *component_observed_paths =
+            ibus_component_get_observed_paths (component);
+        observed_paths = g_list_concat (observed_paths,
+                                        component_observed_paths);
+    }
+
+    for (p = observed_paths; p != NULL; p = p->next) {
+        IBusObservedPath *path = (IBusObservedPath *) p->data;
+        GFile *file = g_file_new_for_path (path->path);
+        if (g_hash_table_lookup (registry->monitor_table,file) == NULL) {
+            GFileMonitor *monitor;
+            GError *error;
+
+            error = NULL;
+            monitor = g_file_monitor (file,
+                                      G_FILE_MONITOR_NONE,
+                                      NULL,
+                                      &error);
+
+            if (monitor != NULL) {
+                g_signal_connect (monitor, "changed",
+                                  G_CALLBACK (_monitor_changed_cb),
+                                  registry);
+
+                g_hash_table_replace (registry->monitor_table,
+                                      g_object_ref (file),
+                                      monitor);
+            } else {
+                g_warning ("Can't monitor directory %s: %s",
+                           path->path,
+                           error->message);
+                g_error_free (error);
+            }
+        }
+        g_object_unref (file);
+    }
+    g_list_free (observed_paths);
 }
 
 gboolean
@@ -632,7 +625,6 @@ bus_registry_is_changed (BusRegistry *registry)
     g_assert (BUS_IS_REGISTRY (registry));
     return (registry->changed != 0);
 }
-#endif
 
 void
 bus_registry_name_owner_changed (BusRegistry *registry,
index cdabec0..2864ed9 100644 (file)
@@ -58,13 +58,13 @@ BusRegistry     *bus_registry_new               (void);
 GList           *bus_registry_get_components    (BusRegistry    *registry);
 
 /**
- * bus_registry_get_components:
+ * bus_registry_get_engines:
  * @returns: a list of all IBusEngineDesc objects available. The caller has to call g_list_free for the returned list.
  */
 GList           *bus_registry_get_engines       (BusRegistry    *registry);
 
 /**
- * bus_registry_get_components:
+ * bus_registry_get_engines_by_language:
  * @language: a language name like 'ja'
  * @returns: a list of IBusEngineDesc objects for the language. The caller has to call g_list_free for the returned list.
  */
@@ -113,11 +113,9 @@ void             bus_registry_name_owner_changed
                                                  const gchar    *old_name,
                                                  const gchar    *new_name);
 
-#ifdef G_THREADS_ENABLED
 void             bus_registry_start_monitor_changes
                                                 (BusRegistry    *registry);
 gboolean         bus_registry_is_changed        (BusRegistry    *registry);
-#endif
 
 G_END_DECLS
 #endif
index 253b6c3..fe8ee2f 100644 (file)
@@ -40,7 +40,6 @@ static gboolean _restart = FALSE;
 static void
 _restart_server (void)
 {
-    extern gchar **g_argv;
     gchar *exe;
     gint fd;
 
index e93a3b0..d3cc20f 100644 (file)
@@ -631,9 +631,9 @@ ibus_component_parse_engines (IBusComponent *component,
 }
 
 static void
-ibus_component_parse_observed_paths (IBusComponent    *component,
-                                    XMLNode         *node,
-                                    gboolean         access_fs)
+ibus_component_parse_observed_paths (IBusComponent *component,
+                                     XMLNode       *node,
+                                     gboolean       access_fs)
 {
     g_assert (IBUS_IS_COMPONENT (component));
     g_assert (node);
@@ -652,8 +652,8 @@ ibus_component_parse_observed_paths (IBusComponent    *component,
 
         if (access_fs && path->is_dir && path->is_exist) {
             component->priv->observed_paths =
-                    g_list_concat(component->priv->observed_paths,
-                                  ibus_observed_path_traverse(path));
+                    g_list_concat (component->priv->observed_paths,
+                                   ibus_observed_path_traverse (path, TRUE));
         }
     }
 }
@@ -790,8 +790,8 @@ ibus_component_add_observed_path (IBusComponent *component,
 
     if (access_fs && p->is_dir && p->is_exist) {
         component->priv->observed_paths =
-                g_list_concat(component->priv->observed_paths,
-                              ibus_observed_path_traverse(p));
+                g_list_concat (component->priv->observed_paths,
+                               ibus_observed_path_traverse (p, TRUE));
     }
 }
 
@@ -826,3 +826,10 @@ ibus_component_check_modification (IBusComponent *component)
     }
     return FALSE;
 }
+
+GList *
+ibus_component_get_observed_paths (IBusComponent *component)
+{
+    g_assert (IBUS_IS_COMPONENT (component));
+    return g_list_copy (component->priv->observed_paths);
+}
index 8f35b94..3f4b3b3 100644 (file)
@@ -306,6 +306,18 @@ void             ibus_component_output_engines  (IBusComponent  *component,
  */
 gboolean         ibus_component_check_modification
                                                 (IBusComponent  *component);
+
+/**
+ * ibus_component_get_observed_paths:
+ * @component: An IBusComponent.
+ * @returns: (transfer container) (element-type IBusObservedPath): A
+ * newly allocated GList that contains observed paths.
+ *
+ * Get the observed paths of this component.
+ */
+GList           *ibus_component_get_observed_paths
+                                                (IBusComponent *component);
+
 G_END_DECLS
 #endif
 
index b3b82a0..a63ee8d 100644 (file)
@@ -185,7 +185,8 @@ ibus_observed_path_fill_stat (IBusObservedPath *path)
 }
 
 GList *
-ibus_observed_path_traverse (IBusObservedPath *path)
+ibus_observed_path_traverse (IBusObservedPath *path,
+                             gboolean          dir_only)
 {
     g_assert (IBUS_IS_OBSERVED_PATH (path));
 
@@ -206,10 +207,13 @@ ibus_observed_path_traverse (IBusObservedPath *path)
         sub->path = g_build_filename (path->path, name, NULL);
 
         ibus_observed_path_fill_stat (sub);
-        paths = g_list_append (paths, sub);
-
-        if (sub->is_exist && sub->is_dir)
-            paths = g_list_concat (paths, ibus_observed_path_traverse (sub));
+        if (sub->is_exist && sub->is_dir) {
+            paths = g_list_append (paths, sub);
+            paths = g_list_concat (paths,
+                                   ibus_observed_path_traverse (sub, dir_only));
+        } else if (!dir_only) {
+            paths = g_list_append (paths, sub);
+        }
     }
     g_dir_close (dir);
 
index e137d39..1aefa50 100644 (file)
@@ -116,12 +116,14 @@ IBusObservedPath    *ibus_observed_path_new                 (const gchar
 /**
  * ibus_observed_path_traverse:
  * @path: An IBusObservedPath.
+ * @dir_only: Only looks for subdirs, not files
  * @returns: (element-type IBusObservedPath): A newly allocate GList which holds content in path; NULL if @path is not directory.
  *
  * Recursively traverse the path and put the files and subdirectory in to a newly allocated
  * GLists, if the @path is a directory. Otherwise returns NULL.
  */
-GList               *ibus_observed_path_traverse            (IBusObservedPath   *path);
+GList               *ibus_observed_path_traverse            (IBusObservedPath   *path,
+                                                             gboolean            dir_only);
 
 /**
  * ibus_observed_path_check_modification: