[gnome-software: 18/20] lib: Drop support for plugin job timeouts

[Milan Crha <mcrha src gnome org>]

commit 01942006cce9ef0cda6d32d047af260e1889e8ea
Author: Philip Withnall <pwithnall endlessos org>
Date:   Tue Jul 5 15:50:46 2022 +0100

    lib: Drop support for plugin job timeouts
    
    This basically reverts the changes from
    https://bugzilla.gnome.org/show_bug.cgi?id=784251. Rather than force a
    timeout on plugins which are misbehaving, it’s better to fix the plugins
    so they don’t time out.
    
    If plugins are communicating with another service (such as snapd or a
    D-Bus service), then timeouts can be implemented inside the plugin for
    when that communication fails.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>

 lib/gs-plugin-job-private.h  |   1 -
 lib/gs-plugin-job.c          |  31 -------------
 lib/gs-plugin-job.h          |   2 -
 lib/gs-plugin-loader.c       | 102 -------------------------------------------
 plugins/dummy/gs-self-test.c |  27 ------------
 5 files changed, 163 deletions(-)
---
diff --git a/lib/gs-plugin-job-private.h b/lib/gs-plugin-job-private.h
index 895f70ab2..9cdbbf3db 100644
--- a/lib/gs-plugin-job-private.h
+++ b/lib/gs-plugin-job-private.h
@@ -26,7 +26,6 @@ void                   gs_plugin_job_remove_refine_flags      (GsPluginJob    *self,
 gboolean                gs_plugin_job_get_interactive          (GsPluginJob    *self);
 gboolean                gs_plugin_job_get_propagate_error      (GsPluginJob    *self);
 guint                   gs_plugin_job_get_max_results          (GsPluginJob    *self);
-guint                   gs_plugin_job_get_timeout              (GsPluginJob    *self);
 GsAppListSortFunc       gs_plugin_job_get_sort_func            (GsPluginJob    *self,
                                                                 gpointer       *user_data_out);
 const gchar            *gs_plugin_job_get_search               (GsPluginJob    *self);
diff --git a/lib/gs-plugin-job.c b/lib/gs-plugin-job.c
index b5ce7ab85..a40980fec 100644
--- a/lib/gs-plugin-job.c
+++ b/lib/gs-plugin-job.c
@@ -22,7 +22,6 @@ typedef struct
        gboolean                 interactive;
        gboolean                 propagate_error;
        guint                    max_results;
-       guint                    timeout;
        GsPlugin                *plugin;
        GsPluginAction           action;
        GsAppListSortFunc        sort_func;
@@ -45,7 +44,6 @@ enum {
        PROP_LIST,
        PROP_FILE,
        PROP_MAX_RESULTS,
-       PROP_TIMEOUT,
        PROP_PROPAGATE_ERROR,
        PROP_LAST
 };
@@ -74,8 +72,6 @@ gs_plugin_job_to_string (GsPluginJob *self)
                g_string_append_printf (str, " with interactive=True");
        if (priv->propagate_error)
                g_string_append_printf (str, " with propagate-error=True");
-       if (priv->timeout > 0)
-               g_string_append_printf (str, " with timeout=%u", priv->timeout);
 
        if (priv->max_results > 0)
                g_string_append_printf (str, " with max-results=%u", priv->max_results);
@@ -209,22 +205,6 @@ gs_plugin_job_get_max_results (GsPluginJob *self)
        return priv->max_results;
 }
 
-void
-gs_plugin_job_set_timeout (GsPluginJob *self, guint timeout)
-{
-       GsPluginJobPrivate *priv = gs_plugin_job_get_instance_private (self);
-       g_return_if_fail (GS_IS_PLUGIN_JOB (self));
-       priv->timeout = timeout;
-}
-
-guint
-gs_plugin_job_get_timeout (GsPluginJob *self)
-{
-       GsPluginJobPrivate *priv = gs_plugin_job_get_instance_private (self);
-       g_return_val_if_fail (GS_IS_PLUGIN_JOB (self), 0);
-       return priv->timeout;
-}
-
 void
 gs_plugin_job_set_action (GsPluginJob *self, GsPluginAction action)
 {
@@ -381,9 +361,6 @@ gs_plugin_job_get_property (GObject *obj, guint prop_id, GValue *value, GParamSp
        case PROP_MAX_RESULTS:
                g_value_set_uint (value, priv->max_results);
                break;
-       case PROP_TIMEOUT:
-               g_value_set_uint (value, priv->timeout);
-               break;
        case PROP_PROPAGATE_ERROR:
                g_value_set_boolean (value, priv->propagate_error);
                break;
@@ -426,9 +403,6 @@ gs_plugin_job_set_property (GObject *obj, guint prop_id, const GValue *value, GP
        case PROP_MAX_RESULTS:
                gs_plugin_job_set_max_results (self, g_value_get_uint (value));
                break;
-       case PROP_TIMEOUT:
-               gs_plugin_job_set_timeout (self, g_value_get_uint (value));
-               break;
        case PROP_PROPAGATE_ERROR:
                gs_plugin_job_set_propagate_error (self, g_value_get_boolean (value));
                break;
@@ -508,11 +482,6 @@ gs_plugin_job_class_init (GsPluginJobClass *klass)
                                   G_PARAM_READWRITE);
        g_object_class_install_property (object_class, PROP_MAX_RESULTS, pspec);
 
-       pspec = g_param_spec_uint ("timeout", NULL, NULL,
-                                  0, G_MAXUINT, 60,
-                                  G_PARAM_READWRITE | G_PARAM_CONSTRUCT);
-       g_object_class_install_property (object_class, PROP_TIMEOUT, pspec);
-
        pspec = g_param_spec_boolean ("propagate-error", NULL, NULL,
                                      FALSE,
                                      G_PARAM_READWRITE);
diff --git a/lib/gs-plugin-job.h b/lib/gs-plugin-job.h
index bec7e8d89..dcdbc85e5 100644
--- a/lib/gs-plugin-job.h
+++ b/lib/gs-plugin-job.h
@@ -45,8 +45,6 @@ void           gs_plugin_job_set_propagate_error      (GsPluginJob    *self,
                                                         gboolean        propagate_error);
 void            gs_plugin_job_set_max_results          (GsPluginJob    *self,
                                                         guint           max_results);
-void            gs_plugin_job_set_timeout              (GsPluginJob    *self,
-                                                        guint           timeout);
 void            gs_plugin_job_set_sort_func            (GsPluginJob    *self,
                                                         GsAppListSortFunc sort_func,
                                                         gpointer        user_data);
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 74e7c1e47..08eaa3022 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -192,8 +192,6 @@ typedef struct {
        GPtrArray                       *catlist;
        GsPluginJob                     *plugin_job;
        gboolean                         anything_ran;
-       guint                            timeout_id;
-       gboolean                         timeout_triggered;
        gchar                           **tokens;
 } GsPluginLoaderHelper;
 
@@ -255,8 +253,6 @@ gs_plugin_loader_helper_free (GsPluginLoaderHelper *helper)
        }
 
        g_object_unref (helper->plugin_loader);
-       if (helper->timeout_id != 0)
-               g_source_remove (helper->timeout_id);
        if (helper->plugin_job != NULL)
                g_object_unref (helper->plugin_job);
        if (helper->catlist != NULL)
@@ -705,20 +701,6 @@ gs_plugin_loader_call_vfunc (GsPluginLoaderHelper *helper,
 
        /* failed */
        if (!ret) {
-               /* we returned cancelled, but this was because of a timeout,
-                * so re-create error, throwing the plugin under the bus */
-               if (helper->timeout_triggered &&
-                   (g_error_matches (error_local, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_CANCELLED) ||
-                    g_error_matches (error_local, G_IO_ERROR, G_IO_ERROR_CANCELLED))) {
-                       g_debug ("converting cancelled to timeout");
-                       g_clear_error (&error_local);
-                       g_set_error (&error_local,
-                                    GS_PLUGIN_ERROR,
-                                    GS_PLUGIN_ERROR_TIMED_OUT,
-                                    "Timeout was reached as %s took "
-                                    "too long to return results",
-                                    gs_plugin_get_name (plugin));
-               }
                return gs_plugin_error_handle_failure (helper,
                                                        plugin,
                                                        error_local,
@@ -3695,75 +3677,6 @@ gs_plugin_loader_process_in_thread_pool_cb (gpointer data,
        g_object_unref (task);
 }
 
-/* This needs to err on the side of being fast, rather than perfectly accurate.
- * False positives (saying the program is running under gdb when it isn’t) are
- * not OK. False negatives (saying the program is not running under gdb when it
- * is) are OK. */
-static gboolean
-is_running_under_gdb (void)
-{
-       g_autofree gchar *status = NULL;
-       gsize status_len = 0;
-       const gchar *tracer_pid, *end;
-
-       /* Look for a line of the form:
-        * ```
-        * TracerPid:   748899
-        * ```
-        * or
-        * ```
-        * TracerPid:   0
-        * ```
-        * in `/proc/self/status`. If it’s 0, the process is not being traced. */
-       if (!g_file_get_contents ("/proc/self/status", &status, &status_len, NULL))
-               return FALSE;
-
-       tracer_pid = g_strstr_len (status, status_len, "TracerPid:");
-       if (tracer_pid == NULL)
-               return FALSE;
-
-       end = status + status_len;
-
-       /* Find the number. */
-       for (tracer_pid += strlen ("TracerPid:");
-            tracer_pid < end &&
-            g_ascii_isspace (*tracer_pid);
-            tracer_pid++);
-
-       if (tracer_pid >= end)
-               return FALSE;
-
-       return (*tracer_pid != '0');
-}
-
-static gboolean
-gs_plugin_loader_job_timeout_cb (gpointer user_data)
-{
-       GTask *task = G_TASK (user_data);
-       GsPluginLoaderHelper *helper = g_task_get_task_data (task);
-
-       /* Don’t impose timeouts if running under gdb. */
-       if (is_running_under_gdb ()) {
-               g_debug ("Not cancelling job %s even though it took longer "
-                        "than %u seconds, as running under gdb",
-                        helper->function_name,
-                        gs_plugin_job_get_timeout (helper->plugin_job));
-               helper->timeout_id = 0;
-               return G_SOURCE_REMOVE;
-       }
-
-       /* call the cancellable */
-       g_debug ("cancelling job %s as it took longer than %u seconds",
-                helper->function_name,
-                gs_plugin_job_get_timeout (helper->plugin_job));
-       g_cancellable_cancel (g_task_get_cancellable (task));
-
-       /* failed */
-       helper->timeout_triggered = TRUE;
-       helper->timeout_id = 0;
-       return G_SOURCE_REMOVE;
-}
-
 static void
 gs_plugin_loader_cancelled_cb (GCancellable *cancellable,
                                gpointer      user_data)
@@ -4088,21 +4001,6 @@ job_process_cb (GTask *task)
        g_task_set_check_cancellable (task, FALSE);
        g_task_set_return_on_cancel (task, FALSE);
 
-       /* set up a hang handler */
-       switch (action) {
-       case GS_PLUGIN_ACTION_GET_ALTERNATES:
-       case GS_PLUGIN_ACTION_SEARCH_PROVIDES:
-               if (gs_plugin_job_get_timeout (plugin_job) > 0) {
-                       helper->timeout_id =
-                               g_timeout_add_seconds (gs_plugin_job_get_timeout (plugin_job),
-                                                      gs_plugin_loader_job_timeout_cb,
-                                                      task);
-               }
-               break;
-       default:
-               break;
-       }
-
        switch (action) {
        case GS_PLUGIN_ACTION_INSTALL:
        case GS_PLUGIN_ACTION_UPDATE:
diff --git a/plugins/dummy/gs-self-test.c b/plugins/dummy/gs-self-test.c
index 3ffe925ee..5f025a195 100644
--- a/plugins/dummy/gs-self-test.c
+++ b/plugins/dummy/gs-self-test.c
@@ -492,30 +492,6 @@ gs_plugins_dummy_search_alternate_func (GsPluginLoader *plugin_loader)
        g_assert_cmpint (gs_app_get_kind (app_tmp), ==, AS_COMPONENT_KIND_DESKTOP_APP);
 }
 
-static void
-gs_plugins_dummy_hang_func (GsPluginLoader *plugin_loader)
-{
-       g_autoptr(GCancellable) cancellable = g_cancellable_new ();
-       g_autoptr(GError) error = NULL;
-       g_autoptr(GsAppList) list = NULL;
-       g_autoptr(GsPluginJob) plugin_job = NULL;
-       g_autoptr(GsAppQuery) query = NULL;
-       const gchar *keywords[2] = { NULL, };
-
-       /* drop all caches */
-       gs_utils_rmtree (g_getenv ("GS_SELF_TEST_CACHEDIR"), NULL);
-       gs_test_reinitialise_plugin_loader (plugin_loader, allowlist, NULL);
-
-       /* get search result based on addon keyword */
-       keywords[0] = "hang";
-       query = gs_app_query_new ("keywords", keywords, NULL);
-       plugin_job = gs_plugin_job_list_apps_new (query, GS_PLUGIN_LIST_APPS_FLAGS_NONE);
-       list = gs_plugin_loader_job_process (plugin_loader, plugin_job, cancellable, &error);
-       gs_test_flush_main_context ();
-       g_assert_error (error, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_TIMED_OUT);
-       g_assert (list == NULL);
-}
-
 static void
 gs_plugins_dummy_search_invalid_func (GsPluginLoader *plugin_loader)
 {
@@ -969,9 +945,6 @@ main (int argc, char **argv)
        g_test_add_data_func ("/gnome-software/plugins/dummy/search-alternate",
                              plugin_loader,
                              (GTestDataFunc) gs_plugins_dummy_search_alternate_func);
-       g_test_add_data_func ("/gnome-software/plugins/dummy/hang",
-                             plugin_loader,
-                             (GTestDataFunc) gs_plugins_dummy_hang_func);
        g_test_add_data_func ("/gnome-software/plugins/dummy/search{invalid}",
                              plugin_loader,
                              (GTestDataFunc) gs_plugins_dummy_search_invalid_func);