From e9533a3f5d96fe01b99feffe1a04e037f5fa7239 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Sun, 4 Apr 2021 15:09:50 +0200 Subject: Qt: stop capture syntax worker thread on exit Instead of creating endless loop and synchronizing using QWaitCondition, execute the syntax worker check in its thread by emitting signal. The syntax worker thread affinity is set to worker thread so the slots handling takes place within the worker thread context. --- ui/qt/capture_filter_syntax_worker.cpp | 163 ++++++++++++++------------------- ui/qt/capture_filter_syntax_worker.h | 8 +- ui/qt/widgets/capture_filter_edit.cpp | 21 +++-- ui/qt/widgets/capture_filter_edit.h | 3 + 4 files changed, 89 insertions(+), 106 deletions(-) (limited to 'ui/qt') diff --git a/ui/qt/capture_filter_syntax_worker.cpp b/ui/qt/capture_filter_syntax_worker.cpp index aa3b59dc2f..711acb46eb 100644 --- a/ui/qt/capture_filter_syntax_worker.cpp +++ b/ui/qt/capture_filter_syntax_worker.cpp @@ -34,7 +34,7 @@ static QMutex pcap_compile_mtx_; #if 0 #include #include -#define DEBUG_SYNTAX_CHECK(state1, state2) qDebug() << "CF state" << QThread::currentThreadId() << state1 << "->" << state2 << ":" << filter_text_ << ":" << filter +#define DEBUG_SYNTAX_CHECK(state1, state2) qDebug() << "CF state" << QThread::currentThreadId() << state1 << "->" << state2 << ":" << filter #define DEBUG_SLEEP_TIME 5000 // ms #else #define DEBUG_SYNTAX_CHECK(state1, state2) @@ -44,121 +44,100 @@ static QMutex pcap_compile_mtx_; #define DUMMY_SNAPLENGTH 65535 #define DUMMY_NETMASK 0xFF000000 -void CaptureFilterSyntaxWorker::start() { +void CaptureFilterSyntaxWorker::checkFilter(const QString filter) +{ #ifdef HAVE_LIBPCAP - forever { - QString filter; - QSet active_dlts; - QSet active_extcap; - struct bpf_program fcode; - pcap_t *pd; - int pc_err; - enum SyntaxLineEdit::SyntaxState state = SyntaxLineEdit::Valid; - QString err_str; - - data_mtx_.lock(); - while (filter_text_.isEmpty()) { - data_cond_.wait(&data_mtx_); - } + QSet active_dlts; + QSet active_extcap; + struct bpf_program fcode; + pcap_t *pd; + int pc_err; + enum SyntaxLineEdit::SyntaxState state = SyntaxLineEdit::Valid; + QString err_str; - DEBUG_SYNTAX_CHECK("pending", "unknown"); - filter = filter_text_; - filter_text_ = QString(); - data_mtx_.unlock(); + DEBUG_SYNTAX_CHECK("received", "?"); - if (global_capture_opts.num_selected < 1) { - emit syntaxResult(filter, SyntaxLineEdit::Invalid, QString("No interfaces selected")); - DEBUG_SYNTAX_CHECK("unknown", "no interfaces"); - continue; - } + if (global_capture_opts.num_selected < 1) { + emit syntaxResult(filter, SyntaxLineEdit::Invalid, QString("No interfaces selected")); + DEBUG_SYNTAX_CHECK("unknown", "no interfaces"); + return; + } - for (guint if_idx = 0; if_idx < global_capture_opts.all_ifaces->len; if_idx++) { - interface_t *device; + for (guint if_idx = 0; if_idx < global_capture_opts.all_ifaces->len; if_idx++) { + interface_t *device; - device = &g_array_index(global_capture_opts.all_ifaces, interface_t, if_idx); - if (device->selected) { - if (device->if_info.extcap == NULL || strlen(device->if_info.extcap) == 0) { - if (device->active_dlt >= DLT_USER0 && device->active_dlt <= DLT_USER15) { - // Capture filter for DLT_USER is unknown - state = SyntaxLineEdit::Deprecated; - err_str = "Unable to check capture filter"; - } else { - active_dlts.insert(device->active_dlt); - } + device = &g_array_index(global_capture_opts.all_ifaces, interface_t, if_idx); + if (device->selected) { + if (device->if_info.extcap == NULL || strlen(device->if_info.extcap) == 0) { + if (device->active_dlt >= DLT_USER0 && device->active_dlt <= DLT_USER15) { + // Capture filter for DLT_USER is unknown + state = SyntaxLineEdit::Deprecated; + err_str = "Unable to check capture filter"; } else { - active_extcap.insert(if_idx); + active_dlts.insert(device->active_dlt); } + } else { + active_extcap.insert(if_idx); } } + } - foreach (gint dlt, active_dlts.values()) { - pcap_compile_mtx_.lock(); - pd = pcap_open_dead(dlt, DUMMY_SNAPLENGTH); - if (pd == NULL) - { - //don't have ability to verify capture filter - break; - } + foreach(gint dlt, active_dlts.values()) { + pcap_compile_mtx_.lock(); + pd = pcap_open_dead(dlt, DUMMY_SNAPLENGTH); + if (pd == NULL) + { + //don't have ability to verify capture filter + break; + } #ifdef PCAP_NETMASK_UNKNOWN - pc_err = pcap_compile(pd, &fcode, filter.toUtf8().data(), 1 /* Do optimize */, PCAP_NETMASK_UNKNOWN); + pc_err = pcap_compile(pd, &fcode, filter.toUtf8().data(), 1 /* Do optimize */, PCAP_NETMASK_UNKNOWN); #else - pc_err = pcap_compile(pd, &fcode, filter.toUtf8().data(), 1 /* Do optimize */, 0); + pc_err = pcap_compile(pd, &fcode, filter.toUtf8().data(), 1 /* Do optimize */, 0); #endif #if DEBUG_SLEEP_TIME > 0 - QThread::msleep(DEBUG_SLEEP_TIME); + QThread::msleep(DEBUG_SLEEP_TIME); #endif - if (pc_err) { + if (pc_err) { + DEBUG_SYNTAX_CHECK("unknown", "known bad"); + state = SyntaxLineEdit::Invalid; + err_str = pcap_geterr(pd); + } else { + DEBUG_SYNTAX_CHECK("unknown", "known good"); + } + pcap_close(pd); + + pcap_compile_mtx_.unlock(); + + if (state == SyntaxLineEdit::Invalid) break; + } + // If it's already invalid, don't bother to check extcap + if (state != SyntaxLineEdit::Invalid) { + foreach(guint extcapif, active_extcap.values()) { + interface_t *device; + gchar *error = NULL; + + device = &g_array_index(global_capture_opts.all_ifaces, interface_t, extcapif); + extcap_filter_status status = extcap_verify_capture_filter(device->name, filter.toUtf8().constData(), &error); + if (status == EXTCAP_FILTER_VALID) { + DEBUG_SYNTAX_CHECK("unknown", "known good"); + } else if (status == EXTCAP_FILTER_INVALID) { DEBUG_SYNTAX_CHECK("unknown", "known bad"); state = SyntaxLineEdit::Invalid; - err_str = pcap_geterr(pd); + err_str = error; + break; } else { - DEBUG_SYNTAX_CHECK("unknown", "known good"); - } - pcap_close(pd); - - pcap_compile_mtx_.unlock(); - - if (state == SyntaxLineEdit::Invalid) break; - } - // If it's already invalid, don't bother to check extcap - if (state != SyntaxLineEdit::Invalid) { - foreach (guint extcapif, active_extcap.values()) { - interface_t *device; - gchar *error = NULL; - - device = &g_array_index(global_capture_opts.all_ifaces, interface_t, extcapif); - extcap_filter_status status = extcap_verify_capture_filter(device->name, filter.toUtf8().constData(), &error); - if (status == EXTCAP_FILTER_VALID) { - DEBUG_SYNTAX_CHECK("unknown", "known good"); - } else if (status == EXTCAP_FILTER_INVALID) { - DEBUG_SYNTAX_CHECK("unknown", "known bad"); - state = SyntaxLineEdit::Invalid; - err_str = error; - break; - } else { - state = SyntaxLineEdit::Deprecated; - err_str = "Unable to check capture filter"; - } - g_free (error); + state = SyntaxLineEdit::Deprecated; + err_str = "Unable to check capture filter"; } + g_free(error); } - emit syntaxResult(filter, state, err_str); - - DEBUG_SYNTAX_CHECK("known", "idle"); } -#endif // HAVE_LIBPCAP -} + emit syntaxResult(filter, state, err_str); -void CaptureFilterSyntaxWorker::checkFilter(const QString &filter) -{ -#ifdef HAVE_LIBPCAP - QMutexLocker ml(&data_mtx_); - /* Ruthlessly clobber the current state. */ - filter_text_ = filter; - DEBUG_SYNTAX_CHECK("received", "?"); - data_cond_.wakeOne(); + DEBUG_SYNTAX_CHECK("known", "idle"); #else emit syntaxResult(filter, SyntaxLineEdit::Deprecated, QString("Syntax checking unavailable")); #endif // HAVE_LIBPCAP diff --git a/ui/qt/capture_filter_syntax_worker.h b/ui/qt/capture_filter_syntax_worker.h index eae59f3a1f..5b061363b4 100644 --- a/ui/qt/capture_filter_syntax_worker.h +++ b/ui/qt/capture_filter_syntax_worker.h @@ -20,15 +20,9 @@ class CaptureFilterSyntaxWorker : public QObject public: CaptureFilterSyntaxWorker(QObject *parent = 0) : QObject(parent) {} - void checkFilter(const QString &filter); public slots: - void start(); - -private: - QMutex data_mtx_; - QWaitCondition data_cond_; - QString filter_text_; + void checkFilter(const QString filter); signals: void syntaxResult(QString filter, int state, QString err_msg); diff --git a/ui/qt/widgets/capture_filter_edit.cpp b/ui/qt/widgets/capture_filter_edit.cpp index 1854e63969..ed79cce005 100644 --- a/ui/qt/widgets/capture_filter_edit.cpp +++ b/ui/qt/widgets/capture_filter_edit.cpp @@ -218,21 +218,28 @@ CaptureFilterEdit::CaptureFilterEdit(QWidget *parent, bool plain) : #endif } - QThread *syntax_thread = new QThread; + syntax_thread_ = new QThread; syntax_worker_ = new CaptureFilterSyntaxWorker; - syntax_worker_->moveToThread(syntax_thread); + syntax_worker_->moveToThread(syntax_thread_); connect(wsApp, &WiresharkApplication::appInitialized, this, &CaptureFilterEdit::updateBookmarkMenu); connect(wsApp, &WiresharkApplication::captureFilterListChanged, this, &CaptureFilterEdit::updateBookmarkMenu); - connect(syntax_thread, &QThread::started, syntax_worker_, &CaptureFilterSyntaxWorker::start); - connect(syntax_thread, &QThread::started, this, + connect(syntax_thread_, &QThread::started, this, static_cast(&CaptureFilterEdit::checkFilter)); connect(syntax_worker_, &CaptureFilterSyntaxWorker::syntaxResult, this, &CaptureFilterEdit::setFilterSyntaxState); - connect(syntax_thread, &QThread::finished, syntax_worker_, &CaptureFilterSyntaxWorker::deleteLater); - syntax_thread->start(); + connect(this, &CaptureFilterEdit::captureFilterChanged, syntax_worker_, &CaptureFilterSyntaxWorker::checkFilter); + syntax_thread_->start(); updateBookmarkMenu(); } +CaptureFilterEdit::~CaptureFilterEdit() +{ + syntax_thread_->quit(); + syntax_thread_->wait(); + delete syntax_thread_; + delete syntax_worker_; +} + void CaptureFilterEdit::paintEvent(QPaintEvent *evt) { SyntaxLineEdit::paintEvent(evt); @@ -366,7 +373,7 @@ void CaptureFilterEdit::checkFilter(const QString& filter) if (empty) { setFilterSyntaxState(filter, Empty, QString()); } else { - syntax_worker_->checkFilter(filter); + emit captureFilterChanged(filter); } } diff --git a/ui/qt/widgets/capture_filter_edit.h b/ui/qt/widgets/capture_filter_edit.h index 5ca8c7e9a1..8d8769695e 100644 --- a/ui/qt/widgets/capture_filter_edit.h +++ b/ui/qt/widgets/capture_filter_edit.h @@ -24,6 +24,7 @@ class CaptureFilterEdit : public SyntaxLineEdit Q_OBJECT public: explicit CaptureFilterEdit(QWidget *parent = 0, bool plain = false); + ~CaptureFilterEdit(); void setConflict(bool conflict = false); // No selections: (QString(), false) // Selections, same filter: (filter, false) @@ -63,11 +64,13 @@ private: StockIconToolButton *clear_button_; StockIconToolButton *apply_button_; CaptureFilterSyntaxWorker *syntax_worker_; + QThread *syntax_thread_; void buildCompletionList(const QString& primitive_word); signals: void captureFilterSyntaxChanged(bool valid); + void captureFilterChanged(const QString filter); void startCapture(); void addBookmark(const QString filter); -- cgit v1.2.3