Rework tapping in Qt dialogs.

Add cf_cb_file_retap_started and cf_cb_file_retap_finished to file.[ch].
Add their associated signals to CaptureFile.

Add registerTapListener and removeTapListeners to WiresharkDialog, which
collect and automatically remove tap listeners. Add beginRetapPackets
and endRetapPackets, which can be used to wrap critical sections so that
we don't delete ourselves while tapping. Don't cancel tapping on close
in WiresharkDialog.

Use beginRetapPackets and endRetapPackets in WiresharkDialog and
FollowStreamDialog. We will likely need to add them elsewhere.

Update comments.

Change-Id: I1788a6ade0817c31aa3419216df96be5e36b2178
Reviewed-on: https://code.wireshark.org/review/10261
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>

21 files changed, 257 insertions, 218 deletions

diff --git a/file.c b/file.c
index 77b59f89a5..ce209ba466 100644
--- a/file.c
+++ b/file.c
@@ -2127,6 +2127,8 @@ cf_retap_packets(capture_file *cf)
     return CF_READ_ABORTED;
   }
 
+  cf_callback_invoke(cf_cb_file_retap_started, cf);
+
   /* Do we have any tap listeners with filters? */
   filtering_tap_listeners = have_filtering_tap_listeners();
 
@@ -2156,6 +2158,8 @@ cf_retap_packets(capture_file *cf)
 
   epan_dissect_cleanup(&callback_args.edt);
 
+  cf_callback_invoke(cf_cb_file_retap_finished, cf);
+
   switch (ret) {
   case PSP_FINISHED:
     /* Completed successfully. */
diff --git a/file.h b/file.h
index b34617c61b..2403e24707 100644
--- a/file.h
+++ b/file.h
@@ -71,6 +71,8 @@ typedef enum {
     cf_cb_file_reload_finished,
     cf_cb_file_rescan_started,
     cf_cb_file_rescan_finished,
+    cf_cb_file_retap_started,
+    cf_cb_file_retap_finished,
     cf_cb_file_fast_save_finished,
     cf_cb_packet_selected,
     cf_cb_packet_unselected,
diff --git a/ui/qt/bluetooth_att_server_attributes_dialog.cpp b/ui/qt/bluetooth_att_server_attributes_dialog.cpp
index 1c89e97a29..4ce9b8cc49 100644
--- a/ui/qt/bluetooth_att_server_attributes_dialog.cpp
+++ b/ui/qt/bluetooth_att_server_attributes_dialog.cpp
@@ -98,27 +98,6 @@ btatt_handle_tap_reset(void *tapinfo_ptr)
         tapinfo->tap_reset(tapinfo);
 }
 
-
-static void
-bluetooth_att_server_attributes_tap(void *data)
-{
-    GString *error_string;
-
-    error_string = register_tap_listener("btatt.handles", data, NULL,
-            0,
-            btatt_handle_tap_reset,
-            btatt_handle_tap_packet,
-            NULL
-            );
-
-    if (error_string != NULL) {
-        simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-                "%s", error_string->str);
-        g_string_free(error_string, TRUE);
-    }
-}
-
-
 BluetoothAttServerAttributesDialog::BluetoothAttServerAttributesDialog(QWidget &parent, CaptureFile &cf) :
     WiresharkDialog(parent, cf),
     ui(new Ui::BluetoothAttServerAttributesDialog)
@@ -142,7 +121,12 @@ BluetoothAttServerAttributesDialog::BluetoothAttServerAttributesDialog(QWidget &
     tapinfo_.tap_reset  = tapReset;
     tapinfo_.ui = this;
 
-    bluetooth_att_server_attributes_tap(&tapinfo_);
+    registerTapListener("btatt.handles", &tapinfo_, NULL,
+                        0,
+                        btatt_handle_tap_reset,
+                        btatt_handle_tap_packet,
+                        NULL
+                        );
 
     cap_file_.retapPackets();
 }
@@ -151,15 +135,11 @@ BluetoothAttServerAttributesDialog::BluetoothAttServerAttributesDialog(QWidget &
 BluetoothAttServerAttributesDialog::~BluetoothAttServerAttributesDialog()
 {
     delete ui;
-
-    remove_tap_listener(&tapinfo_);
 }
 
 
 void BluetoothAttServerAttributesDialog::captureFileClosing()
 {
-    remove_tap_listener(&tapinfo_);
-
     ui->interfaceComboBox->setEnabled(FALSE);
     ui->deviceComboBox->setEnabled(FALSE);
     ui->removeDuplicatesCheckBox->setEnabled(FALSE);
diff --git a/ui/qt/bluetooth_devices_dialog.cpp b/ui/qt/bluetooth_devices_dialog.cpp
index 38ade7e889..cb8271802f 100644
--- a/ui/qt/bluetooth_devices_dialog.cpp
+++ b/ui/qt/bluetooth_devices_dialog.cpp
@@ -75,27 +75,6 @@ bluetooth_device_tap_reset(void *tapinfo_ptr)
         tapinfo->tap_reset(tapinfo);
 }
 
-
-static void
-bluetooth_devices_tap(void *data)
-{
-    GString *error_string;
-
-    error_string = register_tap_listener("bluetooth.device", data, NULL,
-            0,
-            bluetooth_device_tap_reset,
-            bluetooth_device_tap_packet,
-            NULL
-            );
-
-    if (error_string != NULL) {
-        simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
-                "%s", error_string->str);
-        g_string_free(error_string, TRUE);
-    }
-}
-
-
 BluetoothDevicesDialog::BluetoothDevicesDialog(QWidget &parent, CaptureFile &cf) :
     WiresharkDialog(parent, cf),
     ui(new Ui::BluetoothDevicesDialog)
@@ -118,7 +97,12 @@ BluetoothDevicesDialog::BluetoothDevicesDialog(QWidget &parent, CaptureFile &cf)
     tapinfo_.tap_reset  = tapReset;
     tapinfo_.ui = this;
 
-    bluetooth_devices_tap(&tapinfo_);
+    registerTapListener("bluetooth.device", &tapinfo_, NULL,
+                        0,
+                        bluetooth_device_tap_reset,
+                        bluetooth_device_tap_packet,
+                        NULL
+                        );
 
     cap_file_.retapPackets();
 }
@@ -127,15 +111,11 @@ BluetoothDevicesDialog::BluetoothDevicesDialog(QWidget &parent, CaptureFile &cf)
 BluetoothDevicesDialog::~BluetoothDevicesDialog()
 {
     delete ui;
-
-    remove_tap_listener(&tapinfo_);
 }
 
 
 void BluetoothDevicesDialog::captureFileClosing()
 {
-    remove_tap_listener(&tapinfo_);
-
     ui->interfaceComboBox->setEnabled(FALSE);
     ui->showInformationStepsCheckBox->setEnabled(FALSE);
 
diff --git a/ui/qt/capture_file.cpp b/ui/qt/capture_file.cpp
index 340d3e1e46..4ce0fc03ea 100644
--- a/ui/qt/capture_file.cpp
+++ b/ui/qt/capture_file.cpp
@@ -186,7 +186,6 @@ void CaptureFile::captureFileEvent(int event, gpointer data)
         g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_DEBUG, "Callback: Reload finished");
         emit captureFileReloadFinished();
         break;
-
     case(cf_cb_file_rescan_started):
         g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_DEBUG, "Callback: Rescan started");
         emit captureFileRescanStarted();
@@ -195,6 +194,14 @@ void CaptureFile::captureFileEvent(int event, gpointer data)
         g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_DEBUG, "Callback: Rescan finished");
         emit captureFileRescanFinished();
         break;
+    case(cf_cb_file_retap_started):
+        g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_DEBUG, "Callback: Retap started");
+        emit captureFileRetapStarted();
+        break;
+    case(cf_cb_file_retap_finished):
+        g_log(LOG_DOMAIN_MAIN, G_LOG_LEVEL_DEBUG, "Callback: Retap finished");
+        emit captureFileRetapFinished();
+        break;
 
     case(cf_cb_file_fast_save_finished):
         // Ignored for now
diff --git a/ui/qt/capture_file.h b/ui/qt/capture_file.h
index 430933e76f..d31a9868f8 100644
--- a/ui/qt/capture_file.h
+++ b/ui/qt/capture_file.h
@@ -85,6 +85,8 @@ signals:
     void captureFileReloadFinished() const;
     void captureFileRescanStarted() const;
     void captureFileRescanFinished() const;
+    void captureFileRetapStarted() const;
+    void captureFileRetapFinished() const;
     void captureFileClosing() const;
     void captureFileClosed() const;
     void captureFileSaveStarted(const QString &file_path) const;
diff --git a/ui/qt/conversation_dialog.cpp b/ui/qt/conversation_dialog.cpp
index 89576c144a..8f034e641a 100644
--- a/ui/qt/conversation_dialog.cpp
+++ b/ui/qt/conversation_dialog.cpp
@@ -34,7 +34,6 @@
 
 #include <QCheckBox>
 #include <QDialogButtonBox>
-#include <QMessageBox>
 #include <QPushButton>
 
 // To do:
@@ -127,7 +126,6 @@ void ConversationDialog::captureFileClosing()
     // on a live capture file.
     for (int i = 0; i < trafficTableTabWidget()->count(); i++) {
         ConversationTreeWidget *cur_tree = qobject_cast<ConversationTreeWidget *>(trafficTableTabWidget()->widget(i));
-        remove_tap_listener(cur_tree->trafficTreeHash());
         disconnect(cur_tree, SIGNAL(filterAction(QString&,FilterAction::Action,FilterAction::ActionType)),
                    this, SIGNAL(filterAction(QString&,FilterAction::Action,FilterAction::ActionType)));
     }
@@ -174,16 +172,10 @@ bool ConversationDialog::addTrafficTable(register_ct_t* table)
 
     conv_tree->trafficTreeHash()->user_data = conv_tree;
 
-    GString *error_string = register_tap_listener(proto_get_protocol_filter_name(proto_id), conv_tree->trafficTreeHash(), filter, 0,
-                                                  ConversationTreeWidget::tapReset,
-                                                  get_conversation_packet_func(table),
-                                                  ConversationTreeWidget::tapDraw);
-
-    if (error_string) {
-        QMessageBox::warning(this, tr("Conversation %1 failed to register tap listener").arg(table_name),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
-    }
+    registerTapListener(proto_get_protocol_filter_name(proto_id), conv_tree->trafficTreeHash(), filter, 0,
+                        ConversationTreeWidget::tapReset,
+                        get_conversation_packet_func(table),
+                        ConversationTreeWidget::tapDraw);
 
     return true;
 }
diff --git a/ui/qt/endpoint_dialog.cpp b/ui/qt/endpoint_dialog.cpp
index ec477579a3..d9617d5dd7 100644
--- a/ui/qt/endpoint_dialog.cpp
+++ b/ui/qt/endpoint_dialog.cpp
@@ -114,7 +114,6 @@ void EndpointDialog::captureFileClosing()
     // on a live capture file.
     for (int i = 0; i < trafficTableTabWidget()->count(); i++) {
         EndpointTreeWidget *cur_tree = qobject_cast<EndpointTreeWidget *>(trafficTableTabWidget()->widget(i));
-        remove_tap_listener(cur_tree->trafficTreeHash());
         disconnect(cur_tree, SIGNAL(filterAction(QString&,FilterAction::Action,FilterAction::ActionType)),
                    this, SIGNAL(filterAction(QString&,FilterAction::Action,FilterAction::ActionType)));
     }
@@ -159,16 +158,10 @@ bool EndpointDialog::addTrafficTable(register_ct_t *table)
 
     endp_tree->trafficTreeHash()->user_data = endp_tree;
 
-    GString *error_string = register_tap_listener(proto_get_protocol_filter_name(proto_id), endp_tree->trafficTreeHash(), filter, 0,
-                                                  EndpointTreeWidget::tapReset,
-                                                  get_hostlist_packet_func(table),
-                                                  EndpointTreeWidget::tapDraw);
-
-    if (error_string) {
-        QMessageBox::warning(this, tr("Endpoint %1 failed to register tap listener").arg(table_name),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
-    }
+    registerTapListener(proto_get_protocol_filter_name(proto_id), endp_tree->trafficTreeHash(), filter, 0,
+                        EndpointTreeWidget::tapReset,
+                        get_hostlist_packet_func(table),
+                        EndpointTreeWidget::tapDraw);
 
 #ifdef HAVE_GEOIP
     connect(endp_tree, SIGNAL(geoIPStatusChanged()), this, SLOT(tabChanged()));
diff --git a/ui/qt/expert_info_dialog.cpp b/ui/qt/expert_info_dialog.cpp
index 25012f54b1..141e388c91 100644
--- a/ui/qt/expert_info_dialog.cpp
+++ b/ui/qt/expert_info_dialog.cpp
@@ -207,7 +207,6 @@ ExpertInfoDialog::ExpertInfoDialog(QWidget &parent, CaptureFile &capture_file) :
 
 ExpertInfoDialog::~ExpertInfoDialog()
 {
-    remove_tap_listener(this);
     delete ui;
 }
 
@@ -236,24 +235,20 @@ void ExpertInfoDialog::retapPackets()
     if (file_closed_) return;
 
     clearAllData();
-    remove_tap_listener(this);
-
-    GString *error_string = register_tap_listener("expert",
-                                                 this,
-                                                 NULL,
-                                                 TL_REQUIRES_NOTHING,
-                                                 tapReset,
-                                                 tapPacket,
-                                                 tapDraw);
-    if (error_string) {
-        QMessageBox::warning(this, tr("Endpoint expert failed to register tap listener"),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
+    removeTapListeners();
+
+    if (!registerTapListener("expert",
+                             this,
+                             NULL,
+                             TL_REQUIRES_NOTHING,
+                             tapReset,
+                             tapPacket,
+                             tapDraw)) {
         return;
     }
 
     if (ui->limitCheckBox->isChecked()) {
-        error_string = set_tap_dfilter(this, display_filter_.toUtf8().constData());
+        GString *error_string = set_tap_dfilter(this, display_filter_.toUtf8().constData());
         if (error_string) {
             QMessageBox::warning(this, tr("Endpoint expert failed to set filter"),
                                  error_string->str);
@@ -517,7 +512,6 @@ void ExpertInfoDialog::filterActionTriggered()
 
 void ExpertInfoDialog::captureFileClosing()
 {
-    remove_tap_listener(this);
     WiresharkDialog::captureFileClosing();
 }
 
diff --git a/ui/qt/export_object_dialog.cpp b/ui/qt/export_object_dialog.cpp
index 4812486669..2da7bd5a03 100644
--- a/ui/qt/export_object_dialog.cpp
+++ b/ui/qt/export_object_dialog.cpp
@@ -31,9 +31,9 @@
 #include "wireshark_application.h"
 
 #include <QDialogButtonBox>
-#include <QPushButton>
-#include <QMessageBox>
 #include <QFileDialog>
+#include <QMessageBox>
+#include <QPushButton>
 
 extern "C" {
 
@@ -127,7 +127,7 @@ ExportObjectDialog::~ExportObjectDialog()
 {
     delete eo_ui_;
     export_object_list_.eod = NULL;
-    remove_tap_listener((void *)&export_object_list_);
+    removeTapListeners();
 }
 
 void ExportObjectDialog::addObjectEntry(export_object_entry_t *entry)
@@ -172,22 +172,11 @@ void ExportObjectDialog::resetObjects()
 
 void ExportObjectDialog::show()
 {
-    GString *error_msg;
-
     /* Data will be gathered via a tap callback */
-    error_msg = register_tap_listener(tap_name_, (void *)&export_object_list_, NULL, 0,
-                                      eo_reset,
-                                      tap_packet_,
-                                      NULL);
-
-    if (error_msg) {
-        QMessageBox::warning(
-                    this,
-                    tr("Tap registration error"),
-                    QString(tr("Unable to register ")) + name_ + QString(tr(" tap: ")) + error_msg->str,
-                    QMessageBox::Ok
-                    );
-        g_string_free(error_msg, TRUE);
+    if (!registerTapListener(tap_name_, &export_object_list_, NULL, 0,
+                             eo_reset,
+                             tap_packet_,
+                             NULL)) {
         return;
     }
 
diff --git a/ui/qt/follow_stream_dialog.cpp b/ui/qt/follow_stream_dialog.cpp
index 39655f2203..7fa3947a17 100644
--- a/ui/qt/follow_stream_dialog.cpp
+++ b/ui/qt/follow_stream_dialog.cpp
@@ -841,9 +841,9 @@ bool FollowStreamDialog::follow(QString previous_filter, bool use_stream_index)
     follow_stats_t      stats;
     tcp_stream_chunk    sc;
     size_t              nchars;
-    GString *           msg;
     gboolean is_tcp = FALSE, is_udp = FALSE;
 
+    beginRetapPackets();
     resetStream();
 
     if (file_closed_)
@@ -968,13 +968,9 @@ bool FollowStreamDialog::follow(QString previous_filter, bool use_stream_index)
     case FOLLOW_UDP:
     {
         /* data will be passed via tap callback*/
-        msg = register_tap_listener("udp_follow", &follow_info_,
-                                    follow_filter.toUtf8().constData(),
-                                    0, NULL, udp_queue_packet_data, NULL);
-        if (msg) {
-            QMessageBox::critical(this, "Error",
-                               "Can't register udp_follow tap: %1",
-                               msg->str);
+        if (!registerTapListener("udp_follow", &follow_info_,
+                                 follow_filter.toUtf8().constData(),
+                                 0, NULL, udp_queue_packet_data, NULL)) {
             return false;
         }
 
@@ -990,13 +986,9 @@ bool FollowStreamDialog::follow(QString previous_filter, bool use_stream_index)
     }
     case FOLLOW_SSL:
         /* we got ssl so we can follow */
-        msg = register_tap_listener("ssl", &follow_info_,
-                                    follow_filter.toUtf8().constData(), 0,
-                                    NULL, ssl_queue_packet_data, NULL);
-        if (msg)
-        {
-            QMessageBox::critical(this, "Error",
-                          "Can't register ssl tap: %1", msg->str);
+        if (!registerTapListener("ssl", &follow_info_,
+                                 follow_filter.toUtf8().constData(), 0,
+                                 NULL, ssl_queue_packet_data, NULL)) {
             return false;
         }
         break;
@@ -1012,10 +1004,8 @@ bool FollowStreamDialog::follow(QString previous_filter, bool use_stream_index)
 
         break;
     case FOLLOW_UDP:
-        remove_tap_listener(&follow_info_);
-        break;
     case FOLLOW_SSL:
-        remove_tap_listener(&follow_info_);
+        removeTapListeners();
         break;
     }
 
@@ -1235,6 +1225,7 @@ bool FollowStreamDialog::follow(QString previous_filter, bool use_stream_index)
         data_out_file = NULL;
     }
 
+    endRetapPackets();
     return true;
 }
 
diff --git a/ui/qt/response_time_delay_dialog.cpp b/ui/qt/response_time_delay_dialog.cpp
index 193dbbaa80..cbbf8bf072 100644
--- a/ui/qt/response_time_delay_dialog.cpp
+++ b/ui/qt/response_time_delay_dialog.cpp
@@ -26,7 +26,6 @@
 #include "epan/proto.h"
 #include "epan/rtd_table.h"
 
-#include <QMessageBox>
 #include <QTreeWidget>
 
 #include "qt_ui_utils.h"
@@ -243,18 +242,14 @@ void ResponseTimeDelayDialog::fillTree()
     rtd_table_dissector_init(rtd_, &rtd_data.stat_table, NULL, NULL);
     rtd_data.user_data = this;
 
-    QString display_filter = displayFilter();
-    GString *error_string = register_tap_listener(get_rtd_tap_listener_name(rtd_),
+    QByteArray display_filter = displayFilter().toUtf8();
+    if (!registerTapListener(get_rtd_tap_listener_name(rtd_),
                           &rtd_data,
-                          display_filter.toUtf8().constData(),
+                          display_filter.constData(),
                           0,
                           tapReset,
                           get_rtd_packet_func(rtd_),
-                          tapDraw);
-    if (error_string) {
-        QMessageBox::critical(this, tr("Failed to attach to tap \"%1\"").arg(get_rtd_tap_listener_name(rtd_)),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
+                          tapDraw)) {
         free_rtd_table(&rtd_data.stat_table, NULL, NULL);
         reject(); // XXX Stay open instead?
         return;
@@ -269,7 +264,7 @@ void ResponseTimeDelayDialog::fillTree()
     statsTreeWidget()->sortItems(col_type_, Qt::AscendingOrder);
     statsTreeWidget()->setSortingEnabled(true);
 
-    remove_tap_listener(&rtd_data);
+    removeTapListeners();
     free_rtd_table(&rtd_data.stat_table, NULL, NULL);
 }
 
diff --git a/ui/qt/rtp_analysis_dialog.cpp b/ui/qt/rtp_analysis_dialog.cpp
index 2ebdb29bc6..493012da00 100644
--- a/ui/qt/rtp_analysis_dialog.cpp
+++ b/ui/qt/rtp_analysis_dialog.cpp
@@ -449,10 +449,9 @@ RtpAnalysisDialog::RtpAnalysisDialog(QWidget &parent, CaptureFile &cf) :
             this, SLOT(updateWidgets()));
     updateWidgets();
 
-    register_tap_listener("rtp", this, NULL, 0, tapReset, tapPacket, tapDraw);
-
+    registerTapListener("rtp", this, NULL, 0, tapReset, tapPacket, tapDraw);
     cap_file_.retapPackets();
-    remove_tap_listener(this);
+    removeTapListeners();
 
     updateStatistics();
 }
diff --git a/ui/qt/service_response_time_dialog.cpp b/ui/qt/service_response_time_dialog.cpp
index 5219d3ceaf..1c939d9d0b 100644
--- a/ui/qt/service_response_time_dialog.cpp
+++ b/ui/qt/service_response_time_dialog.cpp
@@ -30,7 +30,6 @@
 #include "rpc_service_response_time_dialog.h"
 #include "wireshark_application.h"
 
-#include <QMessageBox>
 #include <QTreeWidget>
 #include <QTreeWidgetItemIterator>
 
@@ -266,19 +265,13 @@ void ServiceResponseTimeDialog::fillTree()
     srt_table_dissector_init(srt_, srt_data.srt_array, NULL, NULL);
 
     QString display_filter = displayFilter();
-    GString *error_string = register_tap_listener(get_srt_tap_listener_name(srt_),
-                          &srt_data,
-                          display_filter.toUtf8().constData(),
-                          0,
-                          tapReset,
-                          get_srt_packet_func(srt_),
-                          tapDraw);
-    if (error_string) {
-        QMessageBox::critical(this, tr("Failed to attach to tap \"%1\"").arg(get_srt_tap_listener_name(srt_)),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
-        g_array_free(srt_data.srt_array, TRUE);
-        srt_data.srt_array = NULL;
+    if (!registerTapListener(get_srt_tap_listener_name(srt_),
+                        &srt_data,
+                        display_filter.toUtf8().constData(),
+                        0,
+                        tapReset,
+                        get_srt_packet_func(srt_),
+                        tapDraw)) {
         reject(); // XXX Stay open instead?
         return;
     }
@@ -297,7 +290,8 @@ void ServiceResponseTimeDialog::fillTree()
     statsTreeWidget()->sortItems(SRT_COLUMN_PROCEDURE, Qt::AscendingOrder);
     statsTreeWidget()->setSortingEnabled(true);
 
-    remove_tap_listener(&srt_data);
+    removeTapListeners();
+
     g_array_free(srt_data.srt_array, TRUE);
 }
 
diff --git a/ui/qt/simple_statistics_dialog.cpp b/ui/qt/simple_statistics_dialog.cpp
index 525153580d..9ed61324aa 100644
--- a/ui/qt/simple_statistics_dialog.cpp
+++ b/ui/qt/simple_statistics_dialog.cpp
@@ -25,7 +25,6 @@
 
 #include "epan/stat_tap_ui.h"
 
-#include <QMessageBox>
 #include <QTreeWidget>
 
 #include "wireshark_application.h"
@@ -252,17 +251,13 @@ void SimpleStatisticsDialog::fillTree()
     stu_->stat_tap_init_cb(stu_, NULL, NULL);
 
     QString display_filter = displayFilter();
-    GString *error_string = register_tap_listener(stu_->tap_name,
-                          &stat_data,
-                          display_filter.toUtf8().constData(),
-                          0,
-                          tapReset,
-                          stu_->packet_func,
-                          tapDraw);
-    if (error_string) {
-        QMessageBox::critical(this, tr("Failed to attach to tap \"%1\"").arg(stu_->tap_name),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
+    if (!registerTapListener(stu_->tap_name,
+                             &stat_data,
+                             display_filter.toUtf8().constData(),
+                             0,
+                             tapReset,
+                             stu_->packet_func,
+                             tapDraw)) {
         free_stat_tables(stu_, NULL, NULL);
         reject(); // XXX Stay open instead?
         return;
@@ -272,7 +267,7 @@ void SimpleStatisticsDialog::fillTree()
 
     tapDraw(&stat_data);
 
-    remove_tap_listener(&stat_data);
+    removeTapListeners();
     free_stat_tables(stu_, NULL, NULL);
 }
 
diff --git a/ui/qt/stats_tree_dialog.cpp b/ui/qt/stats_tree_dialog.cpp
index f4f804f673..4f998568a9 100644
--- a/ui/qt/stats_tree_dialog.cpp
+++ b/ui/qt/stats_tree_dialog.cpp
@@ -111,7 +111,6 @@ void StatsTreeDialog::setupNode(stat_node* node)
 
 void StatsTreeDialog::fillTree()
 {
-    GString *error_string;
     if (!st_cfg_ || file_closed_) return;
 
     QString display_name = gchar_free_to_qstring(stats_tree_get_displayname(st_cfg_->name));
@@ -139,26 +138,22 @@ void StatsTreeDialog::fillTree()
     resize(st_->num_columns*80+80, height());
     statsTreeWidget()->setSortingEnabled(false);
 
-    error_string = register_tap_listener(st_cfg_->tapname,
-                          st_,
-                          st_->filter,
-                          st_cfg_->flags,
-                          resetTap,
-                          stats_tree_packet,
-                          drawTreeItems);
-    if (error_string) {
-        QMessageBox::critical(this, tr("%1 failed to attach to tap").arg(display_name),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
+    if (!registerTapListener(st_cfg_->tapname,
+                             st_,
+                             st_->filter,
+                             st_cfg_->flags,
+                             resetTap,
+                             stats_tree_packet,
+                             drawTreeItems)) {
         reject(); // XXX Stay open instead?
         return;
     }
 
-    cf_retap_packets(cap_file_.capFile());
+    cap_file_.retapPackets();
     drawTreeItems(st_);
 
     statsTreeWidget()->setSortingEnabled(true);
-    remove_tap_listener(st_);
+    removeTapListeners();
 
     st_cfg_->pr = NULL;
 }
diff --git a/ui/qt/tap_parameter_dialog.cpp b/ui/qt/tap_parameter_dialog.cpp
index ba7e4001e6..93e9ec223d 100644
--- a/ui/qt/tap_parameter_dialog.cpp
+++ b/ui/qt/tap_parameter_dialog.cpp
@@ -197,7 +197,7 @@ void TapParameterDialog::setRetapOnShow(bool retap)
 {
     show_timer_->stop();
     if (retap) {
-        show_timer_->singleShot(0, this, SLOT(fillTree()));
+        show_timer_->singleShot(0, this, SLOT(fillTreeWrapper()));
     }
 }
 
@@ -429,6 +429,13 @@ QByteArray TapParameterDialog::getTreeAsString(st_format_type format)
     return ba;
 }
 
+void TapParameterDialog::fillTreeWrapper()
+{
+    ui->applyFilterButton->setEnabled(false);
+    fillTree();
+    ui->applyFilterButton->setEnabled(true);
+}
+
 void TapParameterDialog::drawTreeItems()
 {
     if (ui->statsTreeWidget->model()->rowCount() < expand_all_threshold_) {
@@ -519,11 +526,9 @@ void TapParameterDialog::on_applyFilterButton_clicked()
     if (!ui->displayFilterLineEdit->checkFilter())
         return;
 
-    ui->applyFilterButton->setEnabled(false);
     QString filter = ui->displayFilterLineEdit->text();
     emit updateFilter(filter, true);
-    fillTree();
-    ui->applyFilterButton->setEnabled(true);
+    fillTreeWrapper();
 }
 
 void TapParameterDialog::on_actionCopyToClipboard_triggered()
diff --git a/ui/qt/tap_parameter_dialog.h b/ui/qt/tap_parameter_dialog.h
index d79a30d75c..55915d2c8d 100644
--- a/ui/qt/tap_parameter_dialog.h
+++ b/ui/qt/tap_parameter_dialog.h
@@ -110,6 +110,7 @@ private:
 private slots:
     // Called by the constructor. The subclass should tap packets here.
     virtual void fillTree() = 0;
+    void fillTreeWrapper();
 
     void on_applyFilterButton_clicked();
     void on_actionCopyToClipboard_triggered();
diff --git a/ui/qt/wireshark_dialog.cpp b/ui/qt/wireshark_dialog.cpp
index d816a4b553..8ddfddc1cd 100644
--- a/ui/qt/wireshark_dialog.cpp
+++ b/ui/qt/wireshark_dialog.cpp
@@ -19,20 +19,19 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-/*
- * @file General dialog base class
- *
- * Base class which provides convenience methods for dialogs that handle
- * capture files. "General" is a misnomer but we already have a class named
- * "CaptureFileDialog".
- */
-
 #include "config.h"
 
 #include <glib.h>
 
+#include "cfile.h"
+
+#include <epan/packet.h>
+#include <epan/tap.h>
+
 #include "wireshark_dialog.h"
 
+#include <QMessageBox>
+
 #include "wireshark_application.h"
 
 // To do:
@@ -42,31 +41,40 @@
 WiresharkDialog::WiresharkDialog(QWidget &, CaptureFile &capture_file) :
     QDialog(NULL, Qt::Window),
     cap_file_(capture_file),
-    file_closed_(false)
+    file_closed_(false),
+    retap_depth_(0),
+    dialog_closed_(false)
 {
     setWindowIcon(wsApp->normalIcon());
+    setWindowTitleFromSubtitle();
+
+    connect(&cap_file_, SIGNAL(captureFileRetapStarted()), this, SLOT(beginRetapPackets()));
+    connect(&cap_file_, SIGNAL(captureFileRetapFinished()), this, SLOT(endsRetapPackets()));
     connect(&cap_file_, SIGNAL(captureFileClosing()), this, SLOT(captureFileClosing()));
     connect(&cap_file_, SIGNAL(captureFileClosed()), this, SLOT(captureFileClosing()));
-    setWindowTitleFromSubtitle();
 }
 
 void WiresharkDialog::accept()
 {
-    // Cancel any taps in progress.
-    cap_file_.setCaptureStopFlag();
-    // We need to make sure our destructor is called.
-    deleteLater();
     QDialog::accept();
+
+    // Cancel any taps in progress?
+    // cap_file_.setCaptureStopFlag();
+    removeTapListeners();
+    dialog_closed_ = true;
+    tryDeleteLater();
 }
 
 // XXX Should we do this in WiresharkDialog?
 void WiresharkDialog::reject()
 {
-    // Cancel any taps in progress.
-    cap_file_.setCaptureStopFlag();
-    // We need to make sure our destructor is called.
-    deleteLater();
     QDialog::reject();
+
+    // Cancel any taps in progress?
+    // cap_file_.setCaptureStopFlag();
+    removeTapListeners();
+    dialog_closed_ = true;
+    tryDeleteLater();
 }
 
 
@@ -82,13 +90,54 @@ void WiresharkDialog::setWindowTitleFromSubtitle()
     QDialog::setWindowTitle(title);
 }
 
+// See if we can destroy ourselves. The user may have clicked "Close" while
+// we were deep in the bowels of a routine that retaps packets. Track our
+// tapping state using retap_depth_ and our closed state using dialog_closed_.
+void WiresharkDialog::tryDeleteLater()
+{
+    // In many of our subclasses, if the user clicks "Apply" followed by
+    // "Close" we can end up calling fillTree after calling tryDeleteLater.
+    // Disconnecting our slots here prevents that (in Qt5 at least).
+    disconnect();
+    if (retap_depth_ < 1 && dialog_closed_) deleteLater();
+}
+
 void WiresharkDialog::updateWidgets()
 {
     setWindowTitleFromSubtitle();
 }
 
+bool WiresharkDialog::registerTapListener(const char *tap_name, void *tap_data, const char *filter, guint flags, void(*tap_reset)(void *), gboolean(*tap_packet)(void *, struct _packet_info *, struct epan_dissect *, const void *), void(*tap_draw)(void *))
+{
+    GString *error_string = register_tap_listener(tap_name, tap_data, filter, flags,
+                                                  tap_reset, tap_packet, tap_draw);
+    if (error_string) {
+        QMessageBox::warning(this, tr("Failed to attach to tap \"%1\"").arg(tap_name),
+                             error_string->str);
+        g_string_free(error_string, TRUE);
+        return false;
+    }
+
+    tap_listeners_ << tap_data;
+    return true;
+}
+
+void WiresharkDialog::endRetapPackets()
+{
+    retap_depth_--;
+    tryDeleteLater();
+}
+
+void WiresharkDialog::removeTapListeners()
+{
+    while (!tap_listeners_.isEmpty()) {
+        remove_tap_listener(tap_listeners_.takeFirst());
+    }
+}
+
 void WiresharkDialog::captureFileClosing()
 {
+    removeTapListeners();
     file_closed_ = true;
     setWindowTitleFromSubtitle();
     updateWidgets();
diff --git a/ui/qt/wireshark_dialog.h b/ui/qt/wireshark_dialog.h
index 33ca4100d9..d90be2104b 100644
--- a/ui/qt/wireshark_dialog.h
+++ b/ui/qt/wireshark_dialog.h
@@ -22,6 +22,17 @@
 #ifndef WIRESHARK_DIALOG_H
 #define WIRESHARK_DIALOG_H
 
+/*
+ * @file General dialog base class
+ *
+ * Base class which provides convenience methods for dialogs that handle
+ * capture files. "General" is a misnomer but we already have a class named
+ * "CaptureFileDialog". Suggestions for a better name from
+ * https://code.wireshark.org/review/#/c/9739/:
+ * BaseCaptureDialog, CaptureHelperDialog (or rename CaptureFileDialog to something else - WiresharkFileDialog).
+ * TapDialog might make sense as well.
+ */
+
 #include "capture_file.h"
 
 #include <QDialog>
@@ -37,26 +48,92 @@ public:
 signals:
 
 public slots:
+    /**
+     * @brief Mark the start of a code block that retaps packets. If the user
+     * closes the dialog while tapping, the dialog will not be destroyed until
+     * endRetapPackets is called.
+     *
+     * This is automatically called when tapping begins, but might need to be
+     * called explicilty if any member functions are called or variables are
+     * accessed after tapping is finished.
+     */
+
+    void beginRetapPackets() { retap_depth_++; }
+    /**
+     * @brief Mark the end of a code block that retaps packets. If the user
+     * has closed the dialog it will be desroyed at this point.
+     *
+     * This is automatically called when tapping ends, but might need to be
+     * called explicilty if any member functions are called or variables are
+     * accessed after tapping is finished.
+     */
+    void endRetapPackets();
 
 protected:
     virtual void keyPressEvent(QKeyEvent *event) { QDialog::keyPressEvent(event); }
     virtual void accept();
     virtual void reject();
 
+    /**
+     * @brief Set the window subtitle, e.g. "Foo Timeouts". The subtitle and
+     * file name will be added to the dialog window title.
+     * @param subtitle The subtitle to add. It should be unique, short, and
+     * descriptive.
+     */
     void setWindowSubtitle(const QString &subtitle);
     const QString &windowSubtitle() { return subtitle_; }
     virtual void updateWidgets();
 
+    // Capture file and tapping
     CaptureFile &cap_file_;
+    /**
+     * @brief Convenience wrapper for register_tap_listener. Tap
+     * listeners registered via this function are automatically
+     * removed during destruction. They can also be explicitly
+     * removed using remove_tap_listener or removeTapListeners.
+     *
+     * Shows a warning dialog if registration is unsuccessful.
+     * @param tap_name A registered tap name.
+     * @param tap_data A unique pointer. Usually 'this'.
+     * @param filter A display filter.
+     * @param flags See register_tap_listener.
+     * @param tap_reset Reset callback.
+     * @param tap_packet Per-packet callback.
+     * @param tap_draw Draw callback.
+     */
+    bool registerTapListener(const char *tap_name, void *tap_data,
+                        const char *filter, guint flags,
+                        void (*tap_reset)(void *tapdata),
+                        gboolean (*tap_packet)(void *tapdata, struct _packet_info *pinfo, struct epan_dissect *edt, const void *data),
+                        void (*tap_draw)(void *tap_data));
+
+    /**
+     * @brief Remove all tap listeners registered via registerTapListener.
+     */
+    void removeTapListeners();
+
+    /**
+     * @brief true if the file has been closed, false otherwise.
+     */
+    // XXX Needs a getter?
     bool file_closed_;
 
 protected slots:
+    /**
+     * @brief Called when the capture file is about to close. This can be
+     * used to enable or disable widgets according to the state of
+     * file_closed_.
+     */
     virtual void captureFileClosing();
 
 private:
     void setWindowTitleFromSubtitle();
+    void tryDeleteLater();
 
     QString subtitle_;
+    QList<void *> tap_listeners_;
+    int retap_depth_;
+    int dialog_closed_;
 
 private slots:
 };
diff --git a/ui/qt/wlan_statistics_dialog.cpp b/ui/qt/wlan_statistics_dialog.cpp
index 3855de86fe..8759e77025 100644
--- a/ui/qt/wlan_statistics_dialog.cpp
+++ b/ui/qt/wlan_statistics_dialog.cpp
@@ -27,7 +27,6 @@
 
 #include <epan/dissectors/packet-ieee80211.h>
 
-#include <QMessageBox>
 #include <QTreeWidget>
 #include <QTreeWidgetItem>
 
@@ -596,24 +595,20 @@ const QString WlanStatisticsDialog::filterExpression()
 
 void WlanStatisticsDialog::fillTree()
 {
-    GString *error_string = register_tap_listener("wlan",
-                                                 this,
-                                                 NULL,
-                                                 TL_REQUIRES_NOTHING,
-                                                 tapReset,
-                                                 tapPacket,
-                                                 tapDraw);
-    if (error_string) {
-        QMessageBox::warning(this, tr("Endpoint expert failed to register tap listener"),
-                             error_string->str);
-        g_string_free(error_string, TRUE);
+    if (!registerTapListener("wlan",
+                             this,
+                             NULL,
+                             TL_REQUIRES_NOTHING,
+                             tapReset,
+                             tapPacket,
+                             tapDraw)) {
         reject();
         return;
     }
 
     cap_file_.retapPackets();
     tapDraw(this);
-    remove_tap_listener(this);
+    removeTapListeners();
 
     for (int i = 0; i < statsTreeWidget()->topLevelItemCount(); i++) {
         QTreeWidgetItem *ti = statsTreeWidget()->topLevelItem(i);