Fix a crash which can occur if a user hits "Capture:Options" immediately followed by "Capture:Start"

Fixes Bug #4645: https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4645

Essentially: The "Capture Start" code "interrupted" the "Capture Options" code and attempted
 to use not yet initialized "Capture Options" state.

svn path=/trunk/; revision=35633

1 files changed, 57 insertions, 9 deletions

diff --git a/gtk/capture_dlg.c b/gtk/capture_dlg.c
index 1fbce6f8ea..22a9e3e7d6 100644
--- a/gtk/capture_dlg.c
+++ b/gtk/capture_dlg.c
@@ -159,11 +159,15 @@
 
 /*
  * Keep a static pointer to the current "Capture Options" window, if
- * any, so that if somebody tries to do "Capture:Start" while there's
+ * any, so that if somebody tries to do "Capture:Options" while there's
  * already a "Capture Options" window up, we just pop up the existing
  * one, rather than creating a new one.
+ * Also: Capture:Start obtains info from the "Capture Options" window
+ *       if it exists and if its creation is complete.
  */
-static GtkWidget *cap_open_w;
+static GtkWidget *cap_open_w = NULL;
+static gboolean   cap_open_complete;  /* valid only if cap_open_w != NULL */
+
 static GHashTable *cap_settings_history=NULL;
 
 #ifdef HAVE_PCAP_REMOTE
@@ -368,9 +372,9 @@ set_if_capabilities(gboolean monitor_mode_changed)
     if (iftype >= CAPTURE_IFREMOTE)
       if_list = (GList *) g_object_get_data(G_OBJECT(cap_open_w), E_CAP_IF_LIST_KEY);
     else
-      if_list = capture_interface_list(&err, NULL);
+      if_list = capture_interface_list(&err, NULL);   /* Warning: see capture_prep_cb() */
 #else
-    if_list = capture_interface_list(&err, NULL);
+    if_list = capture_interface_list(&err, NULL);     /* Warning: see capture_prep_cb() */
 #endif
     if (if_list != NULL) {
       /*
@@ -909,7 +913,7 @@ update_interface_list()
                                         &err, &err_str);
     g_object_set_data(G_OBJECT(cap_open_w), E_CAP_IF_LIST_KEY, if_list);
   } else {
-    if_list = capture_interface_list(&err, &err_str);
+    if_list = capture_interface_list(&err, &err_str);   /* Warning: see capture_prep_cb() */
     g_object_set_data(G_OBJECT(cap_open_w), E_CAP_IF_LIST_KEY, NULL);
   }
 
@@ -1557,6 +1561,38 @@ capture_filter_compile_cb(GtkWidget *w _U_, gpointer user_data _U_)
 #endif
 
 /* show capture prepare (options) dialog */
+
+/* XXX: Warning:
+        Note that capture_interface_list() is called directly (or indirectly) during the
+         creation of (and changes to) the capture options dialog window.
+
+        Also note that capture_interface_list() indirectly runs the gtk main loop temporarily
+         to process queued events (which may include button-presses, key-presses, etc).
+         (This is done while awaiting a response from dumpcap which is invoked to obtain
+          the capture interface list).
+        This means other Wireshark callbacks can be invoked while the capture options window
+         is being created or updated (in effect an "interrupt" can occur).
+
+        Needless to say, "race conditions" may occur in "interrupt" code which depends upon the exact
+        state of the capture options dialog window and which may be invoked during the
+        creation of (or changes to) the capture options dialog window.
+
+        For example: if a user hits "Capture:Options" and then immediately hits "Capture:Start",
+         capture_start_cb() may be invoked before capture_prep_cb() has been completed (i.e., during
+         a call to capture_interface_list() in the code which creates the capture options window).
+        capture_start_cb() depends upon certain properties of the capture options window having been
+         initialized and thus fails if the properties have not (yet) been initialized.
+
+        An interlock has been added to handle this particular situation;
+        Ideally a more general solution should be implemented since it's probably difficult
+         (if not nearly impossible) to identify all the possible "race conditions".
+
+        ? Prevent the temporary running of the gtk main loop in cases wherein dumpcap is invoked for a
+          simple request/reply ? (e.g., capture_interface_list()) ??
+
+        ? Other ??
+*/
+
 void
 capture_prep_cb(GtkWidget *w _U_, gpointer d _U_)
 {
@@ -1642,11 +1678,11 @@ capture_prep_cb(GtkWidget *w _U_, gpointer d _U_)
   /* use user-defined title if preference is set */
   cap_title = create_user_window_title("Wireshark: Capture Options");
 
+  cap_open_complete = FALSE;
   cap_open_w = dlg_window_new(cap_title);
   g_free(cap_title);
 
   tooltips = gtk_tooltips_new();
-
 #ifdef HAVE_PCAP_REMOTE
   if (global_capture_opts.src_type == CAPTURE_IFREMOTE) {
     if_list = get_remote_interface_list(global_capture_opts.remote_host,
@@ -1665,18 +1701,18 @@ capture_prep_cb(GtkWidget *w _U_, gpointer d _U_)
         g_free (global_capture_opts.iface_descr);
         global_capture_opts.iface_descr = NULL;
       }
-      if_list = capture_interface_list(&err, &err_str);
+      if_list = capture_interface_list(&err, &err_str);   /* Warning: see capture_prep_cb() */
       global_capture_opts.src_type = CAPTURE_IFLOCAL;
       g_object_set_data(G_OBJECT(cap_open_w), E_CAP_IF_LIST_KEY, NULL);
     } else {
       g_object_set_data(G_OBJECT(cap_open_w), E_CAP_IF_LIST_KEY, if_list);
     }
   } else {
-    if_list = capture_interface_list(&err, &err_str);
+    if_list = capture_interface_list(&err, &err_str);   /* Warning: see capture_prep_cb() */
     g_object_set_data(G_OBJECT(cap_open_w), E_CAP_IF_LIST_KEY, NULL);
   }
 #else
-  if_list = capture_interface_list(&err, &err_str);
+  if_list = capture_interface_list(&err, &err_str);   /* Warning: see capture_prep_cb() */
 #endif
   if (if_list == NULL && err == CANT_GET_INTERFACE_LIST) {
     simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", err_str);
@@ -1746,6 +1782,9 @@ capture_prep_cb(GtkWidget *w _U_, gpointer d _U_)
        make the one from the preferences file the default */
     if_device = g_strdup(prefs.capture_device);
     global_capture_opts.iface = g_strdup(get_if_name(if_device));
+    /* Warning: see capture_prep_cb() */
+    /* XXX: Could the following code be changed to use the if_list obtained above instead  */
+    /*      of maybe calling capture_interface_list() again ?                           */
     global_capture_opts.iface_descr = get_interface_descriptive_name(global_capture_opts.iface);
     g_free(if_device);
   }
@@ -2437,6 +2476,8 @@ capture_prep_cb(GtkWidget *w _U_, gpointer d _U_)
 
   gtk_widget_show_all(cap_open_w);
   window_present(cap_open_w);
+
+  cap_open_complete = TRUE;   /* "Capture:Start" is now OK */
 }
 
 /* everythings prepared, now it's really time to start the capture */
@@ -2531,6 +2572,13 @@ capture_start_cb(GtkWidget *w _U_, gpointer d _U_)
      */
     gboolean success;
 
+    /* Determine if "capture start" while building of the "capture options" window */
+    /*  is in progress. If so, ignore the "capture start.                          */
+    /* XXX: Would it be better/cleaner for the "capture options" window code to    */
+    /*      disable the capture start button temporarily ?                         */
+    if (cap_open_complete == FALSE) {
+      return;  /* Building options window: ignore "capture start" */
+    }
     success = capture_dlg_prep(cap_open_w);
     window_destroy(GTK_WIDGET(cap_open_w));
     if (!success)