Revert "Move fill_in_local_interfaces to a thread."

Calling scan_local_interfaces ends up calling fork via extcap. Doing so
from a thread is ill-adivsed:

https://rachelbythebay.com/w/2014/08/16/forkenv/
http://www.linuxprogrammingblog.com/threads-and-fork-think-twice-before-using-them

This reverts commit 5b4894b795.

Revert "fix compilation without pcap." as well.
This reverts commit 51300b3c83.

Change-Id: Ic80582b52398c44af73c6d74dbb3216c4d1b37fc
Reviewed-on: https://code.wireshark.org/review/24772
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>
This commit is contained in:
Gerald Combs 2017-12-11 14:41:58 -08:00
parent 10e9846252
commit 34b62c60bf
8 changed files with 101 additions and 103 deletions

View File

@ -13,10 +13,6 @@
#include <stdio.h>
#include <stdlib.h>
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
#include "capture_opts.h"
#endif
#ifdef HAVE_LIBPCAP
#include <string.h>
@ -25,6 +21,7 @@
#include <glib.h>
#include "capture_opts.h"
#include "ringbuffer.h"
#include <wsutil/clopts_common.h>
@ -37,9 +34,8 @@
#include "ui/filter_files.h"
static gboolean capture_opts_output_to_pipe(const char *save_file, gboolean *is_pipe);
#endif
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
void
capture_opts_init(capture_options *capture_opts)
{
@ -50,9 +46,7 @@ capture_opts_init(capture_options *capture_opts)
capture_opts->default_options.descr = NULL;
capture_opts->default_options.cfilter = NULL;
capture_opts->default_options.has_snaplen = FALSE;
#ifdef HAVE_LIBPCAP
capture_opts->default_options.snaplen = WTAP_MAX_PACKET_SIZE_STANDARD;
#endif
capture_opts->default_options.linktype = -1; /* use interface default */
capture_opts->default_options.promisc_mode = TRUE;
capture_opts->default_options.if_type = IF_WIRED;
@ -109,9 +103,8 @@ capture_opts_init(capture_options *capture_opts)
capture_opts->has_file_interval = FALSE;
capture_opts->file_interval = 60; /* 1 min */
capture_opts->has_ring_num_files = FALSE;
#ifdef HAVE_LIBPCAP
capture_opts->ring_num_files = RINGBUFFER_MIN_NUM_FILES;
#endif
capture_opts->has_autostop_files = FALSE;
capture_opts->autostop_files = 1;
capture_opts->has_autostop_packets = FALSE;
@ -125,9 +118,7 @@ capture_opts_init(capture_options *capture_opts)
capture_opts->output_to_pipe = FALSE;
capture_opts->capture_child = FALSE;
}
#endif
#ifdef HAVE_LIBPCAP
void
capture_opts_cleanup(capture_options *capture_opts)
{

View File

@ -5,7 +5,19 @@
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0+
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#ifndef __CAPTURE_GLOBALS_H__
@ -17,10 +29,7 @@
extern "C" {
#endif /* __cplusplus */
// This should probably be behind an API.
extern capture_options global_capture_opts;
// Should this be an element of capture_options?
extern GMutex global_capture_opts_mtx;
#ifdef __cplusplus
}

View File

@ -5,7 +5,19 @@
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0+
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#include "config.h"
@ -54,8 +66,6 @@ commandline_param_info_t global_commandline_info;
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
capture_options global_capture_opts;
// Initialized in fill_in_local_interfaces_start.
GMutex global_capture_opts_mtx;
#endif
void

View File

@ -2172,7 +2172,6 @@ main(int argc, char *argv[])
capture_session_init(&global_capture_session, &cfile);
#endif
fill_in_local_interfaces_start();
init_report_message(vfailure_alert_box, vwarning_alert_box,
open_failure_alert_box, read_failure_alert_box,
@ -2275,8 +2274,7 @@ main(int argc, char *argv[])
#ifdef HAVE_LIBPCAP
splash_update(RA_INTERFACES, NULL, (gpointer)splash_win);
fill_in_local_interfaces_wait(main_window_update);
g_mutex_lock(&global_capture_opts_mtx);
fill_in_local_interfaces(main_window_update);
if (global_commandline_info.list_link_layer_types)
caps_queries |= CAPS_QUERY_LINK_TYPES;
@ -2613,7 +2611,6 @@ main(int argc, char *argv[])
if (!global_commandline_info.start_capture && !global_capture_opts.default_options.cfilter) {
global_capture_opts.default_options.cfilter = g_strdup(get_conn_cfilter());
}
g_mutex_unlock(&global_capture_opts_mtx);
#else /* HAVE_LIBPCAP */
show_main_window(FALSE);
check_and_warn_user_startup(global_commandline_info.cf_name);

View File

@ -1276,6 +1276,7 @@ welcome_new(void)
g_object_set_data(G_OBJECT(welcome_hb), CAPTURE_VIEW, topic_capture_to_fill);
#ifdef HAVE_LIBPCAP
fill_in_local_interfaces(main_window_update);
fill_capture_box();
/* capture help topic */

View File

@ -6,7 +6,19 @@
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* SPDX-License-Identifier: GPL-2.0+
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#include "config.h"
@ -101,14 +113,18 @@ scan_local_interfaces(void (*update_cb)(void))
GString *ip_str;
interface_options *interface_opts;
gboolean found = FALSE;
static gboolean running = FALSE;
GHashTable *selected_devices;
/* scan_local_interfaces internally calls update_cb to process UI events
to avoid stuck UI while running possibly slow operations. A side effect
of this is that new interface changes can be detected before completing
the last one.
This return avoids recursive scan_local_interfaces operation. */
if (!g_mutex_trylock(&global_capture_opts_mtx)) return;
if (running) {
/* scan_local_interfaces internally calls update_cb to process UI events
to avoid stuck UI while running possibly slow operations. A side effect
of this is that new interface changes can be detected before completing
the last one.
This return avoids recursive scan_local_interfaces operation. */
return;
}
running = TRUE;
/*
* Clear list of known interfaces (all_ifaces) that will be re-discovered on
@ -375,21 +391,16 @@ scan_local_interfaces(void (*update_cb)(void))
}
g_hash_table_destroy(selected_devices);
g_mutex_unlock(&global_capture_opts_mtx);
running = FALSE;
}
// XXX Copied from register.c
#define CB_WAIT_TIME (150 * 1000) // microseconds
static GThread *local_if_thread;
static GAsyncQueue *local_interface_done_q;
/*
* Get the global interface list. Generate it if we haven't done so
* already. This can be quite time consuming the first time, so
* record how long it takes in the info log.
*/
static void *
fill_in_local_interfaces_worker(void *arg _U_)
void
fill_in_local_interfaces(void(*update_cb)(void))
{
GTimeVal start_time;
GTimeVal end_time;
@ -402,7 +413,7 @@ fill_in_local_interfaces_worker(void *arg _U_)
if (!initialized) {
/* do the actual work */
scan_local_interfaces(NULL);
scan_local_interfaces(update_cb);
initialized = TRUE;
}
/* log how long it took */
@ -411,34 +422,6 @@ fill_in_local_interfaces_worker(void *arg _U_)
((end_time.tv_usec - start_time.tv_usec) / 1e6));
g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_INFO, "fill_in_local_interfaces() ends, taking %.3fs", elapsed);
g_async_queue_push(local_interface_done_q, GINT_TO_POINTER(TRUE));
return NULL;
}
#endif
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
void
fill_in_local_interfaces_start(void)
{
#ifdef HAVE_LIBPCAP
if (!local_interface_done_q) {
g_mutex_init(&global_capture_opts_mtx);
local_interface_done_q = g_async_queue_new();
}
local_if_thread = g_thread_new("fill_in_local_interfaces_worker", &fill_in_local_interfaces_worker, NULL);
#endif
}
#endif
#ifdef HAVE_LIBPCAP
void
fill_in_local_interfaces_wait(void(*update_cb)(void))
{
while (!g_async_queue_timeout_pop(local_interface_done_q, CB_WAIT_TIME)) {
update_cb();
}
g_thread_join(local_if_thread);
local_if_thread = NULL;
}
void

View File

@ -2,7 +2,23 @@
* Declarations of routines to manage the global list of interfaces and to
* update widgets/windows displaying items from those lists
*
* SPDX-License-Identifier: GPL-2.0+
* Wireshark - Network traffic analyzer
* By Gerald Combs <gerald@wireshark.org>
* Copyright 1998 Gerald Combs
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version 2
* of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
#ifndef __IFACE_LISTS_H__
@ -12,14 +28,6 @@
extern "C" {
#endif /* __cplusplus */
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
/*
* Update the global interface list in the background. Create it if we
* haven't done so already.
*/
extern void fill_in_local_interfaces_start(void);
#endif
#ifdef HAVE_LIBPCAP
/*
* Used when sorting an interface list into alphabetical order by
@ -28,14 +36,15 @@ extern void fill_in_local_interfaces_start(void);
extern gint if_list_comparator_alph(const void *first_arg, const void *second_arg);
/*
* Update the global interface list in the foreground.
* Get the global interface list. Generate it if we haven't
* done so already.
*/
extern void scan_local_interfaces(void (*update_cb)(void));
extern void fill_in_local_interfaces(void(*update_cb)(void));
/*
* Wait for the global interface list creation to finish.
* Update the global interface list.
*/
extern void fill_in_local_interfaces_wait(void(*update_cb)(void));
extern void scan_local_interfaces(void (*update_cb)(void));
/*
* Hide the interfaces

View File

@ -96,10 +96,6 @@
#include "caputils/capture-pcap-util.h"
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
#include "ui/capture_globals.h"
#endif
#include <QMessageBox>
#ifdef _WIN32
@ -473,18 +469,6 @@ int main(int argc, char *qt_argv[])
/* Assemble the run-time version information string */
runtime_info_str = get_runtime_version_info(get_wireshark_runtime_info);
/*
* Set the initial values in the capture options and fill in our
* interfaces. Options might be overwritten by preference settings
* and then again by the command line parameters. Interfaces might
* take a while to discover, so start discovering them as early as
* possible.
*/
#if defined(HAVE_LIBPCAP) || defined(HAVE_EXTCAP)
capture_opts_init(&global_capture_opts);
#endif
fill_in_local_interfaces_start();
/* Create the user profiles directory */
if (create_profiles_dir(&rf_path) == -1) {
simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
@ -565,7 +549,6 @@ int main(int argc, char *qt_argv[])
rf_path, g_strerror(rf_open_errno));
g_free(rf_path);
}
wsApp->applyCustomColorsFromRecent();
// Initialize our language
@ -603,6 +586,12 @@ int main(int argc, char *qt_argv[])
g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_INFO, "set_console_log_handler, elapsed time %" G_GUINT64_FORMAT " us \n", g_get_monotonic_time() - start_time);
#endif
#ifdef HAVE_LIBPCAP
/* Set the initial values in the capture options. This might be overwritten
by preference settings and then again by the command line parameters. */
capture_opts_init(&global_capture_opts);
#endif
init_report_message(vfailure_alert_box, vwarning_alert_box,
open_failure_alert_box, read_failure_alert_box,
write_failure_alert_box);
@ -707,6 +696,10 @@ int main(int argc, char *qt_argv[])
if (global_commandline_info.dfilter != NULL)
dfilter = QString(global_commandline_info.dfilter);
/* Removed thread code:
* https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=9e277ae6154fd04bf6a0a34ec5655a73e5a736a3
*/
timestamp_set_type(recent.gui_time_format);
timestamp_set_precision(recent.gui_time_precision);
timestamp_set_seconds_type (recent.gui_seconds_format);
@ -717,14 +710,11 @@ int main(int argc, char *qt_argv[])
#endif
splash_update(RA_INTERFACES, NULL, NULL);
/* global_capture_opts_mtx isn't strictly necessary but it does make
* global_capture_opts' chain of custody a bit more obvious. */
fill_in_local_interfaces_wait(main_window_update);
g_mutex_lock(&global_capture_opts_mtx);
fill_in_local_interfaces(main_window_update);
if (global_commandline_info.list_link_layer_types)
caps_queries |= CAPS_QUERY_LINK_TYPES;
if (global_commandline_info.list_timestamp_types)
if (global_commandline_info.list_timestamp_types)
caps_queries |= CAPS_QUERY_TIMESTAMP_TYPES;
if (global_commandline_info.start_capture || caps_queries) {
@ -828,22 +818,31 @@ int main(int argc, char *qt_argv[])
/* For update of WindowTitle (When use gui.window_title preference) */
main_w->setWSWindowTitle();
////////
packet_list_enable_color(recent.packet_list_colorize);
g_log(NULL, G_LOG_LEVEL_DEBUG, "FIX: fetch recent color settings");
packet_list_enable_color(TRUE);
////////
////////
if (!color_filters_init(&err_msg, color_filter_add_cb)) {
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", err_msg);
g_free(err_msg);
}
////////
#ifdef HAVE_LIBPCAP
/* if the user didn't supply a capture filter, use the one to filter out remote connections like SSH */
if (!global_commandline_info.start_capture && !global_capture_opts.default_options.cfilter) {
global_capture_opts.default_options.cfilter = g_strdup(get_conn_cfilter());
}
#else /* HAVE_LIBPCAP */
////////
#endif /* HAVE_LIBPCAP */
wsApp->allSystemsGo();
@ -923,7 +922,6 @@ int main(int argc, char *qt_argv[])
global_capture_opts.default_options.cfilter = g_strdup(get_conn_cfilter());
}
}
g_mutex_unlock(&global_capture_opts_mtx);
#endif /* HAVE_LIBPCAP */
// UAT files used in configuration profiles which are used in Qt dialogs