extcap: run --extcap-config in parallel to reduce startup time

While --extcap-interfaces is now run in parallel, --extcap-config (for
each discovered interface) would only run sequentially after that. Make
sure that the latter command also fully parallel and do not wait for all
extcap interfaces to be discovered first.

This saves another 80ms startup time on Linux (unoptimized ASAN+Debug).

Change-Id: I303fd8fda647b304d5bdaf048a3d1628ec9e02b4
Ping-Bug: 15295
Reviewed-on: https://code.wireshark.org/review/30773
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 257 insertions, 59 deletions

diff --git a/extcap.c b/extcap.c
index 466f83c958..db6f79ab6b 100644
--- a/extcap.c
+++ b/extcap.c
@@ -90,13 +90,67 @@ typedef struct _extcap_callback_info_t
 /* Callback definition for extcap_foreach */
 typedef gboolean(*extcap_cb_t)(extcap_callback_info_t info_structure);
 
-typedef struct extcap_output_info {
-    char *extcap_path;
-    char *output;
-} extcap_output_info_t;
+/** GThreadPool does not support pushing new work from a thread while waiting
+ * for the thread pool to finish. This data structure tracks ongoing work.
+ * See https://gitlab.gnome.org/GNOME/glib/issues/1598 */
+typedef struct thread_pool {
+    GThreadPool    *pool;
+    gint            count;  /**< Number of tasks that have not finished. */
+    GCond           cond;
+    GMutex          data_mutex;
+} thread_pool_t;
+
+/**
+ * Callback definition for extcap_run_all, invoked with a thread pool (to
+ * schedule new tasks), an opaque data parameter, and the output from last task
+ * (or NULL if it failed). The output must be freed by the callback function.
+ * The implementation MUST be thread-safe.
+ */
+typedef void (*extcap_run_cb_t)(thread_pool_t *pool, void *data, char *output);
+
+typedef struct extcap_run_task {
+    const char     *extcap_path;
+    char          **argv;       /**< NULL-terminated arguments list, freed when the task is completed. */
+    extcap_run_cb_t output_cb;
+    void           *data;       /** Parameter to be passed to output_cb. */
+} extcap_run_task_t;
+
+typedef struct extcap_iface_info {
+    char *ifname;                       /**< Interface name. */
+    char *output;                       /**< Output of --extcap-config. */
+} extcap_iface_info_t;
+
+typedef struct extcap_run_extcaps_info {
+    char    *extcap_path;               /**< Extcap program path, MUST be the first member.  */
+    char    *output;                    /**< Output of --extcap-interfaces. */
+    guint   num_interfaces;             /**< Number of discovered interfaces. */
+    extcap_iface_info_t *iface_infos;   /**< Per-interface information. */
+} extcap_run_extcaps_info_t;
+
 
 static void extcap_load_interface_list(void);
 
+static gboolean
+thread_pool_push(thread_pool_t *pool, gpointer data, GError **error)
+{
+    g_mutex_lock(&pool->data_mutex);
+    ++pool->count;
+    g_mutex_unlock(&pool->data_mutex);
+    return g_thread_pool_push(pool->pool, data, error);
+}
+
+static void
+thread_pool_wait(thread_pool_t *pool)
+{
+    g_mutex_lock(&pool->data_mutex);
+    while (pool->count != 0) {
+        g_cond_wait(&pool->cond, &pool->data_mutex);
+    }
+    g_cond_clear(&pool->cond);
+    g_mutex_unlock(&pool->data_mutex);
+    g_mutex_clear(&pool->data_mutex);
+}
+
 GHashTable *
 extcap_loaded_interfaces(void)
 {
@@ -290,11 +344,17 @@ static void extcap_free_array(gchar ** args, int argc)
 }
 
 static void
-extcap_free_output_info_array(extcap_output_info_t *infos, guint count)
+extcap_free_extcaps_info_array(extcap_run_extcaps_info_t *infos, guint count)
 {
     for (guint i = 0; i < count; i++) {
         g_free(infos[i].extcap_path);
         g_free(infos[i].output);
+        for (guint j = 0; j < infos[i].num_interfaces; j++) {
+            extcap_iface_info_t *iface_info = &infos[i].iface_infos[j];
+            g_free(iface_info->ifname);
+            g_free(iface_info->output);
+        }
+        g_free(infos[i].iface_infos);
     }
     g_free(infos);
 }
@@ -320,36 +380,48 @@ extcap_run_one(const extcap_interface *interface, GList *arguments, extcap_cb_t
     extcap_free_array(args, cnt);
 }
 
-/** Thread callback to run an extcap program and save its output. */
+/** Thread callback to run an extcap program and pass its output. */
 static void
 extcap_thread_callback(gpointer data, gpointer user_data)
 {
-    extcap_output_info_t *info = (extcap_output_info_t *)data;
-    GList **arguments_lists = (GList **)user_data;
+    extcap_run_task_t *task = (extcap_run_task_t *)data;
+    thread_pool_t *pool = (thread_pool_t *)user_data;
     const char *dirname = get_extcap_dir();
 
-    // Try normal and fallback arguments.
-    for (int i = 0; i < 2; i++) {
-        GList *arguments = arguments_lists[i];
-        gchar **args = extcap_convert_arguments_to_array(arguments);
-        char *command_output;
-        if (ws_pipe_spawn_sync(dirname, info->extcap_path, g_list_length(arguments), args, &command_output)) {
-            info->output = command_output;
-            extcap_free_array(args, g_list_length(arguments));
-            break;
-        }
-        extcap_free_array(args, g_list_length(arguments));
+    char *command_output;
+    if (ws_pipe_spawn_sync(dirname, task->extcap_path, g_strv_length(task->argv), task->argv, &command_output)) {
+        task->output_cb(pool, task->data, command_output);
+    } else {
+        task->output_cb(pool, task->data, NULL);
     }
+    g_strfreev(task->argv);
+    g_free(task);
+
+    // Notify when all tasks are completed and no new subtasks were created.
+    g_mutex_lock(&pool->data_mutex);
+    if (--pool->count == 0) {
+        g_cond_signal(&pool->cond);
+    }
+    g_mutex_unlock(&pool->data_mutex);
 }
 
 /*
- * Run all extcap programs with the given arguments. Returns an array of outputs
- * for each tool (or NULL if no tools are found) with its size in 'count'.
- * Destroy the returneed value with extcap_free_output_info_array.
+ * Run all extcap programs with the given arguments list, invoke the callback to
+ * do some processing and return the results.
+ *
+ * @param [IN] argv NULL-terminated arguments list.
+ * @param [IN] output_cb Thread callback function that receives the output.
+ * @param [IN] data_size Size of the per-program information that will be returned.
+ * @param [OUT] count Size of the returned array.
+ * @return Array of information or NULL if there are none. The first member of
+ * each element (char *extcap_path) must be freed.
  */
-static extcap_output_info_t *
-extcap_run_all(GList *arguments, GList *fallback_arguments, guint *count)
+static gpointer
+extcap_run_all(const char *argv[], extcap_run_cb_t output_cb, gsize data_size, guint *count)
 {
+    /* Need enough space for at least 'extcap_path'. */
+    g_assert(data_size >= sizeof(char *));
+
     GSList *paths = extcap_get_extcap_paths();
     int i = 0;
 #if GLIB_CHECK_VERSION(2,36,0)
@@ -366,17 +438,32 @@ extcap_run_all(GList *arguments, GList *fallback_arguments, guint *count)
     }
 
     guint64 start_time = g_get_monotonic_time();
-    GList *both_args[] = { arguments, fallback_arguments };
     guint paths_count = g_slist_length(paths);
-    extcap_output_info_t *infos = g_new0(extcap_output_info_t, paths_count);
+    /* GSList is not thread-safe, so pre-allocate an array instead. */
+    gpointer infos = g_malloc0_n(paths_count, data_size);
+
+    thread_pool_t pool;
+    pool.pool = g_thread_pool_new(extcap_thread_callback, &pool, max_threads, FALSE, NULL);
+    pool.count = 0;
+    g_cond_init(&pool.cond);
+    g_mutex_init(&pool.data_mutex);
 
-    GThreadPool *pool = g_thread_pool_new(extcap_thread_callback, both_args, max_threads, FALSE, NULL);
     for (GSList *path = paths; path; path = g_slist_next(path), i++) {
-        infos[i].extcap_path = (char *)path->data;
-        g_thread_pool_push(pool, &infos[i], NULL);
+        extcap_run_task_t *task = g_new0(extcap_run_task_t, 1);
+
+        task->extcap_path = (char *)path->data;
+        task->argv = g_strdupv((char **)argv);
+        task->output_cb = output_cb;
+        task->data = ((char *)infos) + (i * data_size);
+        *((char **)task->data) = (char *)path->data;
+
+        thread_pool_push(&pool, task, NULL);
     }
-    g_thread_pool_free(pool, FALSE, TRUE);  /* Waits for tasks completion. */
-    g_slist_free(paths);
+    g_slist_free(paths);    /* Note: the contents are transferred to 'infos'. */
+
+    /* Wait for all (sub)tasks to complete. */
+    thread_pool_wait(&pool);
+    g_thread_pool_free(pool.pool, FALSE, TRUE);
 
     g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "extcap: completed discovery of %d tools in %.3fms",
             paths_count, (g_get_monotonic_time() - start_time) / 1000.0);
@@ -594,17 +681,6 @@ append_extcap_interface_list(GList *list, char **err_str _U_)
     return list;
 }
 
-static void
-extcap_register_preferences_callback(gpointer key, gpointer value _U_, gpointer user_data _U_)
-{
-    GList *arguments;
-
-    arguments = extcap_get_if_configuration((gchar *)key);
-    /* Memory for prefs are external to an interface, they are part of
-     * extcap core, so the parsed arguments can be freed. */
-    extcap_free_if_configuration(arguments, TRUE);
-}
-
 void extcap_register_preferences(void)
 {
     if (prefs.capture_no_extcap)
@@ -617,11 +693,8 @@ void extcap_register_preferences(void)
         return;
     }
 
-    if ( !_loaded_interfaces || g_hash_table_size(_loaded_interfaces) == 0 )
-        extcap_load_interface_list();
-
-
-    g_hash_table_foreach(_tool_for_ifname, extcap_register_preferences_callback, NULL);
+    // Will load information about extcaps and their supported config.
+    extcap_load_interface_list();
 }
 
 /**
@@ -1761,6 +1834,109 @@ process_new_extcap(const char *extcap, char *output)
 }
 
 
+/** Thread callback to save the output of a --extcap-config call. */
+static void
+extcap_process_config_cb(thread_pool_t *pool _U_, void *data, char *output)
+{
+    extcap_iface_info_t *iface_info = (extcap_iface_info_t *)data;
+    iface_info->output = output;
+}
+
+/**
+ * Thread callback to process discovered interfaces, scheduling more tasks to
+ * retrieve the configuration for each interface. Called once for every extcap
+ * program.
+ */
+static void
+extcap_process_interfaces_cb(thread_pool_t *pool, void *data, char *output)
+{
+    extcap_run_extcaps_info_t *info = (extcap_run_extcaps_info_t *)data;
+    guint i = 0;
+    guint num_interfaces = 0;
+
+    if (!output) {
+        // No interfaces available, nothing to do.
+        return;
+    }
+
+    // Save output for process_new_extcap.
+    info->output = output;
+
+    // Are there any interfaces to query information from?
+    GList *interfaces = extcap_parse_interfaces(output, NULL);
+    for (GList *iface = interfaces; iface; iface = g_list_next(iface)) {
+        extcap_interface *intf = (extcap_interface *)iface->data;
+        if (intf->if_type == EXTCAP_SENTENCE_INTERFACE) {
+            ++num_interfaces;
+        }
+    }
+    if (num_interfaces == 0) {
+        // nothing to do.
+        g_list_free_full(interfaces, extcap_free_interface);
+        return;
+    }
+
+    /* GSList is not thread-safe, so pre-allocate an array instead. */
+    info->iface_infos = g_new0(extcap_iface_info_t, num_interfaces);
+    info->num_interfaces = num_interfaces;
+
+    // Schedule new commands to retrieve the configuration.
+    for (GList *iface = interfaces; iface; iface = g_list_next(iface)) {
+        extcap_interface *intf = (extcap_interface *)iface->data;
+        if (intf->if_type != EXTCAP_SENTENCE_INTERFACE) {
+            continue;
+        }
+
+        const char *argv[] = {
+            EXTCAP_ARGUMENT_CONFIG,
+            EXTCAP_ARGUMENT_INTERFACE,
+            intf->call,
+            NULL
+        };
+        extcap_run_task_t *task = g_new0(extcap_run_task_t, 1);
+        extcap_iface_info_t *iface_info = &info->iface_infos[i++];
+
+        task->extcap_path = info->extcap_path;
+        task->argv = g_strdupv((char **)argv);
+        task->output_cb = extcap_process_config_cb;
+        task->data = iface_info;
+        iface_info->ifname = g_strdup(intf->call);
+
+        thread_pool_push(pool, task, NULL);
+    }
+    g_list_free_full(interfaces, extcap_free_interface);
+}
+
+/**
+ * Thread callback to check whether the new-style --list-interfaces call with an
+ * explicit function succeeded. If not, schedule a call without the new version
+ * argument.
+ */
+static void
+extcap_list_interfaces_cb(thread_pool_t *pool, void *data, char *output)
+{
+    extcap_run_extcaps_info_t *info = (extcap_run_extcaps_info_t *)data;
+
+    if (!output) {
+        /* No output available, schedule a fallback query. */
+        const char *argv[] = {
+            EXTCAP_ARGUMENT_LIST_INTERFACES,
+            NULL
+        };
+        extcap_run_task_t *task = g_new0(extcap_run_task_t, 1);
+
+        task->extcap_path = info->extcap_path;
+        task->argv = g_strdupv((char **)argv);
+        task->output_cb = extcap_process_interfaces_cb;
+        task->data = info;
+
+        thread_pool_push(pool, task, NULL);
+    } else {
+        extcap_process_interfaces_cb(pool, info, output);
+    }
+}
+
+
 /* Handles loading of the interfaces.
  *
  * A list of interfaces can be obtained by calling \ref extcap_loaded_interfaces
@@ -1788,12 +1964,11 @@ extcap_load_interface_list(void)
 
     if (_loaded_interfaces == NULL)
     {
-        GList * arguments = NULL;
-        GList * fallback = NULL;
         int major = 0;
         int minor = 0;
         guint count = 0;
-        extcap_output_info_t *infos;
+        extcap_run_extcaps_info_t *infos;
+        GList *unused_arguments = NULL;
 
         _loaded_interfaces = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, extcap_free_interface_info);
         /* Cleanup lookup table */
@@ -1805,19 +1980,42 @@ extcap_load_interface_list(void)
             _tool_for_ifname = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
         }
 
-        arguments = g_list_append(arguments, g_strdup(EXTCAP_ARGUMENT_LIST_INTERFACES));
-        fallback = g_list_append(fallback, g_strdup(EXTCAP_ARGUMENT_LIST_INTERFACES));
-
         get_ws_version_number(&major, &minor, NULL);
-        arguments = g_list_append(arguments, g_strdup_printf("%s=%d.%d", EXTCAP_ARGUMENT_VERSION, major, minor));
-
-        infos = extcap_run_all(arguments, fallback, &count);
+        char *arg_version = g_strdup_printf("%s=%d.%d", EXTCAP_ARGUMENT_VERSION, major, minor);
+        const char *argv[] = {
+            EXTCAP_ARGUMENT_LIST_INTERFACES,
+            EXTCAP_ARGUMENT_VERSION,
+            NULL
+        };
+        infos = (extcap_run_extcaps_info_t *)extcap_run_all(argv,
+                extcap_list_interfaces_cb, sizeof(extcap_run_extcaps_info_t),
+                &count);
         for (guint i = 0; i < count; i++) {
+            if (!infos[i].output) {
+                continue;
+            }
+
+            // Save new extcap and each discovered interface.
             process_new_extcap(infos[i].extcap_path, infos[i].output);
+            for (guint j = 0; j < infos[i].num_interfaces; j++) {
+                extcap_iface_info_t *iface_info = &infos[i].iface_infos[j];
+
+                if (!iface_info->output) {
+                    continue;
+                }
+
+                extcap_callback_info_t cb_info = {
+                    .ifname = iface_info->ifname,
+                    .output = iface_info->output,
+                    .data = &unused_arguments,
+                };
+                cb_preference(cb_info);
+            }
         }
-        extcap_free_output_info_array(infos, count);
-        g_list_free_full(fallback, g_free);
-        g_list_free_full(arguments, g_free);
+        /* XXX rework cb_preference such that this unused list can be removed. */
+        extcap_free_if_configuration(unused_arguments, TRUE);
+        extcap_free_extcaps_info_array(infos, count);
+        g_free(arg_version);
     }
 }