Skip to content

Commit 5d0e619

Browse files
committed
refactor(threads): stop card event monitor thread immediately on cancel and on confirm completed
WE2-672 Signed-off-by: Mart Somermaa <mrts@users.noreply.github.com>
1 parent 437adea commit 5d0e619

6 files changed

Lines changed: 26 additions & 23 deletions

File tree

src/controller/controller.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessag
5656
void interruptThread(QThread* thread)
5757
{
5858
qDebug() << "Interrupting thread" << uintptr_t(thread);
59-
thread->disconnect();
6059
thread->requestInterruption();
6160
ControllerChildThread::waitForControllerNotify.wakeAll();
6261
}
@@ -333,6 +332,8 @@ void Controller::onDialogCancel()
333332
{
334333
REQUIRE_NON_NULL(window)
335334

335+
stopCardEventMonitorThread();
336+
336337
qDebug() << "User cancelled";
337338

338339
// Schedule application exit when the UI dialog is destroyed.
@@ -352,8 +353,11 @@ void Controller::onPinPadCancel()
352353

353354
void Controller::onCriticalFailure(const QString& error)
354355
{
356+
stopCardEventMonitorThread();
357+
355358
qCritical() << "Exiting due to command" << std::string(commandType())
356359
<< "fatal error:" << error;
360+
357361
_result = makeErrorObject(RESP_TECH_ERROR, error);
358362
writeResponseToStdOut(isInStdinMode, _result, commandType());
359363
disposeUI();
@@ -378,6 +382,7 @@ void Controller::waitForChildThreads()
378382
for (const auto& childThread : childThreads) {
379383
auto thread = childThread.second;
380384
if (thread) {
385+
thread->disconnect();
381386
interruptThread(thread);
382387
// Waiting for PIN input on PIN pad may take a long time, call processEvents() so that
383388
// the UI doesn't freeze.

src/controller/controller.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class Controller : public QObject
7979
void saveChildThreadPtrAndConnectFailureFinish(ControllerChildThread* childThread);
8080
void stopCardEventMonitorThread();
8181
void disposeUI();
82-
void exit();
82+
void exit(); // private slot
8383
void waitForChildThreads();
8484
CommandType commandType();
8585

src/ui/webeiddialog.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "ui_dialog.h"
2828

2929
#include <QButtonGroup>
30+
#include <QCloseEvent>
3031
#include <QDesktopServices>
3132
#include <QFile>
3233
#include <QMessageBox>
@@ -396,6 +397,15 @@ void WebEidDialog::reject()
396397
}
397398
}
398399

400+
void WebEidDialog::closeEvent(QCloseEvent* event)
401+
{
402+
if (closeUnconditionally) {
403+
event->accept();
404+
} else {
405+
WebEidUI::closeEvent(event);
406+
}
407+
}
408+
399409
bool WebEidDialog::event(QEvent* event)
400410
{
401411
if (event->type() == QEvent::LanguageChange) {

src/ui/webeiddialog.hpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
#include "ui.hpp"
2626

27-
#include <QCloseEvent>
27+
class QCloseEvent;
2828

2929
// clang-format off
3030
/**
@@ -76,14 +76,7 @@ class WebEidDialog final : public WebEidUI
7676
bool event(QEvent* event) final;
7777
void reject() final;
7878

79-
void closeEvent(QCloseEvent* event) final
80-
{
81-
if (closeUnconditionally) {
82-
event->accept();
83-
} else {
84-
WebEidUI::closeEvent(event);
85-
}
86-
}
79+
void closeEvent(QCloseEvent* event) final;
8780

8881
void connectOkToCachePinAndEmitSelectedCertificate(const CardCertificateAndPinInfo& certAndPin);
8982

tests/mock-ui/mock-ui.hpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,15 @@ class MockUI : public WebEidUI
5353

5454
void onSigningCertificateMismatch() override {}
5555

56-
void onRetry(const RetriableError) override { emit rejected(); }
56+
void onRetry(const RetriableError) override { reject(); }
5757

5858
void onVerifyPinFailed(const electronic_id::VerifyPinFailed::Status, const qint8) override {}
5959

6060
void onSmartCardStatusUpdate(const RetriableError) override
6161
{
62-
emit rejected();
63-
// Schedule invoking Controller::exit().
62+
reject();
6463
emit destroyed();
6564
}
6665

67-
void quit() final
68-
{
69-
// Schedule invoking Controller::exit().
70-
emit destroyed();
71-
}
66+
void quit() final { emit destroyed(); }
7267
};

tests/tests/main.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ private slots:
7575
void quit_exits();
7676

7777
private:
78-
void runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy, bool waitForQuit = true);
78+
void runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy);
7979
void initGetCert();
8080
void initAuthenticate();
8181
void initCard(bool withSigningScript = true);
@@ -101,7 +101,7 @@ void WebEidTests::statusUpdate_withUnsupportedCard_hasExpectedStatus()
101101
QSignalSpy statusUpdateSpy(controller.get(), &Controller::statusUpdate);
102102

103103
// act
104-
runEventLoopVerifySignalsEmitted(statusUpdateSpy, false);
104+
runEventLoopVerifySignalsEmitted(statusUpdateSpy);
105105

106106
// assert
107107
const auto statusArgument = qvariant_cast<RetriableError>(statusUpdateSpy.first().at(0));
@@ -220,7 +220,7 @@ void WebEidTests::quit_exits()
220220
}
221221
}
222222

223-
void WebEidTests::runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy, bool waitForQuit)
223+
void WebEidTests::runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy)
224224
{
225225
// Waits until Controller emits quit.
226226
QSignalSpy quitSpy(controller.get(), &Controller::quit);
@@ -230,7 +230,7 @@ void WebEidTests::runEventLoopVerifySignalsEmitted(QSignalSpy& actionSpy, bool w
230230

231231
// Run the event loop, verify that signals were emitted.
232232
QVERIFY(actionSpy.wait());
233-
if (waitForQuit && quitSpy.count() < 1) {
233+
if (quitSpy.count() < 1) {
234234
QVERIFY(quitSpy.wait());
235235
}
236236
}

0 commit comments

Comments
 (0)