refactor(threads): stop card event monitor thread immediately on canc…

…el and on critical failure WE2-672 Signed-off-by: Mart Somermaa <[email protected]>
web-eid · Jun 10, 2022 · e32a24a · e32a24a
1 parent 437adea
commit e32a24a
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 23 deletions.
diff --git a/src/controller/controller.cpp b/src/controller/controller.cpp
@@ -56,7 +56,6 @@ QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessag
 void interruptThread(QThread* thread)
 {
     qDebug() << "Interrupting thread" << uintptr_t(thread);
-    thread->disconnect();
     thread->requestInterruption();
     ControllerChildThread::waitForControllerNotify.wakeAll();
 }
@@ -333,6 +332,8 @@ void Controller::onDialogCancel()
 {
     REQUIRE_NON_NULL(window)
 
+    stopCardEventMonitorThread();
+
     qDebug() << "User cancelled";
 
     // Schedule application exit when the UI dialog is destroyed.
@@ -352,8 +353,11 @@ void Controller::onPinPadCancel()
 
 void Controller::onCriticalFailure(const QString& error)
 {
+    stopCardEventMonitorThread();
+
     qCritical() << "Exiting due to command" << std::string(commandType())
                 << "fatal error:" << error;
+
     _result = makeErrorObject(RESP_TECH_ERROR, error);
     writeResponseToStdOut(isInStdinMode, _result, commandType());
     disposeUI();
@@ -378,6 +382,7 @@ void Controller::waitForChildThreads()
     for (const auto& childThread : childThreads) {
         auto thread = childThread.second;
         if (thread) {
+            thread->disconnect();
             interruptThread(thread);
             // Waiting for PIN input on PIN pad may take a long time, call processEvents() so that
             // the UI doesn't freeze.

diff --git a/src/controller/controller.hpp b/src/controller/controller.hpp
@@ -79,7 +79,7 @@ class Controller : public QObject
     void saveChildThreadPtrAndConnectFailureFinish(ControllerChildThread* childThread);
     void stopCardEventMonitorThread();
     void disposeUI();
-    void exit();
+    void exit(); // private slot
     void waitForChildThreads();
     CommandType commandType();
 

diff --git a/src/ui/webeiddialog.cpp b/src/ui/webeiddialog.cpp
@@ -27,6 +27,7 @@
 #include "ui_dialog.h"
 
 #include <QButtonGroup>
+#include <QCloseEvent>
 #include <QDesktopServices>
 #include <QFile>
 #include <QMessageBox>
@@ -396,6 +397,15 @@ void WebEidDialog::reject()
     }
 }
 
+void WebEidDialog::closeEvent(QCloseEvent* event)
+{
+    if (closeUnconditionally) {
+        event->accept();
+    } else {
+        WebEidUI::closeEvent(event);
+    }
+}
+
 bool WebEidDialog::event(QEvent* event)
 {
     if (event->type() == QEvent::LanguageChange) {

diff --git a/src/ui/webeiddialog.hpp b/src/ui/webeiddialog.hpp
@@ -24,7 +24,7 @@
 
 #include "ui.hpp"
 
-#include <QCloseEvent>
+class QCloseEvent;
 
 // clang-format off
 /**
@@ -76,14 +76,7 @@ class WebEidDialog final : public WebEidUI
     bool event(QEvent* event) final;
     void reject() final;
 
-    void closeEvent(QCloseEvent* event) final
-    {
-        if (closeUnconditionally) {
-            event->accept();
-        } else {
-            WebEidUI::closeEvent(event);
-        }
-    }
+    void closeEvent(QCloseEvent* event) final;
 
     void connectOkToCachePinAndEmitSelectedCertificate(const CardCertificateAndPinInfo& certAndPin);
 

diff --git a/tests/mock-ui/mock-ui.hpp b/tests/mock-ui/mock-ui.hpp
@@ -53,20 +53,15 @@ class MockUI : public WebEidUI
 
     void onSigningCertificateMismatch() override {}
 
-    void onRetry(const RetriableError) override { emit rejected(); }
+    void onRetry(const RetriableError) override { reject(); }
 
     void onVerifyPinFailed(const electronic_id::VerifyPinFailed::Status, const qint8) override {}
 
     void onSmartCardStatusUpdate(const RetriableError) override
     {
-        emit rejected();
-        // Schedule invoking Controller::exit().
+        reject();
         emit destroyed();
     }
 
-    void quit() final
-    {
-        // Schedule invoking Controller::exit().
-        emit destroyed();
-    }
+    void quit() final { emit destroyed(); }
 };
diff --git a/tests/tests/main.cpp b/tests/tests/main.cpp
@@ -75,7 +75,7 @@ private slots:
     void quit_exits();
 
 private:
-    void runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy, bool waitForQuit = true);
+    void runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy);
     void initGetCert();
     void initAuthenticate();
     void initCard(bool withSigningScript = true);
@@ -101,7 +101,7 @@ void WebEidTests::statusUpdate_withUnsupportedCard_hasExpectedStatus()
     QSignalSpy statusUpdateSpy(controller.get(), &Controller::statusUpdate);
 
     // act
-    runEventLoopVerifySignalsEmitted(statusUpdateSpy, false);
+    runEventLoopVerifySignalsEmitted(statusUpdateSpy);
 
     // assert
     const auto statusArgument = qvariant_cast<RetriableError>(statusUpdateSpy.first().at(0));
@@ -220,7 +220,7 @@ void WebEidTests::quit_exits()
     }
 }
 
-void WebEidTests::runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy, bool waitForQuit)
+void WebEidTests::runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy)
 {
     // Waits until Controller emits quit.
     QSignalSpy quitSpy(controller.get(), &Controller::quit);
@@ -230,7 +230,7 @@ void WebEidTests::runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy, bool w
 
     // Run the event loop, verify that signals were emitted.
     QVERIFY(actionSpy.wait());
-    if (waitForQuit && quitSpy.count() < 1) {
+    if (quitSpy.count() < 1) {
         QVERIFY(quitSpy.wait());
     }
 }