Skip to content

Fix exception handling in Qt signal/slot system #376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
metsma wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix exception handling in Qt signal/slot system #376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/controller/commands.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class CommandType
NONE = -1
};

CommandType() = default;
constexpr CommandType(const CommandTypeEnum _value) : value(_value) {}
CommandType() noexcept = default;
constexpr CommandType(const CommandTypeEnum _value) noexcept : value(_value) {}

constexpr bool operator==(CommandTypeEnum other) const { return value == other; }
constexpr bool operator!=(CommandTypeEnum other) const { return value != other; }
Expand Down
190 changes: 87 additions & 103 deletions src/controller/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ namespace
const QString RESP_TECH_ERROR = QStringLiteral("ERR_WEBEID_NATIVE_FATAL");
const QString RESP_USER_CANCEL = QStringLiteral("ERR_WEBEID_USER_CANCELLED");

QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage)
QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage) noexcept
{
return {{QStringLiteral("error"),
QVariantMap {
Expand All @@ -53,8 +53,8 @@ QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessag

} // namespace

void Controller::run()
{
void Controller::run() noexcept
try {
// If a command is passed, the application is in command-line mode, else in stdin/stdout mode.
const bool isInCommandLineMode = bool(command);
isInStdinMode = !isInCommandLineMode;
Expand All @@ -63,41 +63,38 @@ void Controller::run()
<< QCoreApplication::applicationVersion() << "running in"
<< (isInStdinMode ? "stdin/stdout" : "command-line") << "mode";

try {
// TODO: cut out stdin mode separate class to avoid bugs in safari mode
if (isInStdinMode) {
// In stdin/stdout mode we first output the version as required by the WebExtension
// and then wait for the actual command.
writeResponseToStdOut(
isInStdinMode,
{{QStringLiteral("version"), QCoreApplication::applicationVersion()}},
"get-version");

command = readCommandFromStdin();
}

REQUIRE_NON_NULL(command)
switch (command->first) {
case CommandType::ABOUT:
WebEidUI::showAboutPage();
return;
case CommandType::QUIT:
// If quit is requested, respond with empty JSON object and quit immediately.
qInfo() << "Quit requested, exiting";
writeResponseToStdOut(true, {}, "quit");
emit quit();
return;
default:
break;
}

commandHandler = getCommandHandler(*command);

startCommandExecution();

} catch (const std::exception& error) {
onCriticalFailure(error.what());
// TODO: cut out stdin mode separate class to avoid bugs in safari mode
if (isInStdinMode) {
// In stdin/stdout mode we first output the version as required by the WebExtension
// and then wait for the actual command.
writeResponseToStdOut(isInStdinMode,
{{QStringLiteral("version"), QCoreApplication::applicationVersion()}},
"get-version");

command = readCommandFromStdin();
}

REQUIRE_NON_NULL(command)
switch (command->first) {
case CommandType::ABOUT:
WebEidUI::showAboutPage();
return;
case CommandType::QUIT:
// If quit is requested, respond with empty JSON object and quit immediately.
qInfo() << "Quit requested, exiting";
writeResponseToStdOut(true, {}, "quit");
emit quit();
return;
default:
break;
}

commandHandler = getCommandHandler(*command);

startCommandExecution();

} catch (const std::exception& error) {
onCriticalFailure(error.what());
}

void Controller::startCommandExecution()
Expand Down Expand Up @@ -129,7 +126,7 @@ void Controller::startCommandExecution()
waitForCardThread->start();
}

void Controller::createWindow()
void Controller::createWindow() noexcept
{
window = WebEidUI::createAndShowDialog(commandHandler->commandType());
connect(this, &Controller::statusUpdate, window, &WebEidUI::onSmartCardStatusUpdate);
Expand All @@ -143,42 +140,34 @@ void Controller::createWindow()
}

void Controller::onCardsAvailable(
const std::vector<electronic_id::ElectronicID::ptr>& availableEids)
{
try {
REQUIRE_NON_NULL(commandHandler)
REQUIRE_NON_NULL(window)
REQUIRE_NOT_EMPTY_CONTAINS_NON_NULL_PTRS(availableEids)

for (const auto& card : availableEids) {
const auto protocol =
card->smartcard().protocol() == SmartCard::Protocol::T0 ? "T=0" : "T=1";
qInfo() << "Card" << card->name() << "in reader" << card->smartcard().readerName()
<< "using protocol" << protocol;
}
const std::vector<electronic_id::ElectronicID::ptr>& availableEids) noexcept
try {
REQUIRE_NON_NULL(commandHandler)
REQUIRE_NON_NULL(window)
REQUIRE_NOT_EMPTY_CONTAINS_NON_NULL_PTRS(availableEids)

window->showWaitingForCardPage(commandHandler->commandType());
for (const auto& card : availableEids) {
const auto protocol =
card->smartcard().protocol() == SmartCard::Protocol::T0 ? "T=0" : "T=1";
qInfo() << "Card" << card->name() << "in reader" << card->smartcard().readerName()
<< "using protocol" << protocol;
}

commandHandler->connectSignals(window);
window->showWaitingForCardPage(commandHandler->commandType());

runCommandHandler(availableEids);
commandHandler->connectSignals(window);

} catch (const std::exception& error) {
onCriticalFailure(error.what());
}
}

void Controller::runCommandHandler(std::vector<ElectronicID::ptr> availableEids) noexcept
try {
auto* commandHandlerRunThread =
new CommandHandlerRunThread(this, *commandHandler, std::move(availableEids));
new CommandHandlerRunThread(this, *commandHandler, availableEids);
connectRetry(commandHandlerRunThread);

commandHandlerRunThread->start();

} catch (const std::exception& error) {
onCriticalFailure(error.what());
}

void Controller::onCertificatesLoaded()
void Controller::onCertificatesLoaded() noexcept
{
auto* cardEventMonitorThread = new CardEventMonitorThread(this, commandType());
connect(this, &Controller::stopCardEventMonitorThread, cardEventMonitorThread,
Expand All @@ -189,7 +178,7 @@ void Controller::onCertificatesLoaded()
cardEventMonitorThread->start();
}

void Controller::disposeUI()
void Controller::disposeUI() noexcept
{
if (window) {
window->disconnect();
Expand All @@ -215,42 +204,32 @@ try {
onCriticalFailure(error.what());
}

void Controller::onCommandHandlerConfirmCompleted(const QVariantMap& res)
{
REQUIRE_NON_NULL(window)

void Controller::onCommandHandlerConfirmCompleted(const QVariantMap& res) noexcept
try {
qDebug() << "Command completed";

// Schedule application exit when the UI dialog is destroyed.
connect(window, &WebEidUI::destroyed, this, &Controller::exit);
_result = res;
writeResponseToStdOut(isInStdinMode, res, commandHandler->commandType());

try {
_result = res;
writeResponseToStdOut(isInStdinMode, res, commandHandler->commandType());
} catch (const std::exception& error) {
qCritical() << "Command" << std::string(commandType())
<< "fatal error while writing response to stdout:" << error;
}

window->quit();
exit();
} catch (const std::exception& error) {
onCriticalFailure(error.what());
}

void Controller::onRetry()
{
try {
// Dispose the UI, it will be re-created during next execution.
disposeUI();
// Command handler signals are still connected, disconnect them so that they can be
// reconnected during next execution.
commandHandler->disconnect();
// Before restarting, wait until child threads finish.
waitForChildThreads();

startCommandExecution();

} catch (const std::exception& error) {
onCriticalFailure(error.what());
}
void Controller::onRetry() noexcept
try {
// Dispose the UI, it will be re-created during next execution.
disposeUI();
// Command handler signals are still connected, disconnect them so that they can be
// reconnected during next execution.
commandHandler->disconnect();
// Before restarting, wait until child threads finish.
waitForChildThreads();

startCommandExecution();

} catch (const std::exception& error) {
onCriticalFailure(error.what());
}

void Controller::connectRetry(const ControllerChildThread* childThread) const
Expand All @@ -262,7 +241,7 @@ void Controller::connectRetry(const ControllerChildThread* childThread) const
connect(childThread, &ControllerChildThread::cancel, this, &Controller::onDialogCancel);
}

void Controller::onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo)
void Controller::onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo) noexcept
{
if (commandHandler) {
onConfirmCommandHandler(certAndPinInfo);
Expand All @@ -272,16 +251,18 @@ void Controller::onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo)
}
}

void Controller::onDialogCancel()
{
void Controller::onDialogCancel() noexcept
try {
qDebug() << "User cancelled";
_result = makeErrorObject(RESP_USER_CANCEL, QStringLiteral("User cancelled"));
writeResponseToStdOut(isInStdinMode, _result, commandType());
exit();
} catch (const std::exception& e) {
onCriticalFailure(e.what());
}

void Controller::onCriticalFailure(const QString& error)
{
void Controller::onCriticalFailure(const QString& error) noexcept
try {
emit stopCardEventMonitorThread();
qCritical() << "Exiting due to command" << std::string(commandType())
<< "fatal error:" << error;
Expand All @@ -299,16 +280,19 @@ void Controller::onCriticalFailure(const QString& error)
writeResponseToStdOut(isInStdinMode, _result, commandType());
}
exit();
} catch (const std::exception& e) {
qCritical() << "Failed to write stdout" << e.what();
exit();
}

void Controller::exit()
void Controller::exit() noexcept
{
disposeUI();
waitForChildThreads();
emit quit();
}

void Controller::waitForChildThreads()
void Controller::waitForChildThreads() noexcept
{
for (auto* thread : findChildren<QThread*>()) {
qDebug() << "Interrupting thread" << uintptr_t(thread);
Expand All @@ -319,7 +303,7 @@ void Controller::waitForChildThreads()
}
}

CommandType Controller::commandType()
CommandType Controller::commandType() const noexcept
{
return commandHandler ? commandHandler->commandType() : CommandType(CommandType::INSERT_CARD);
}
30 changes: 15 additions & 15 deletions src/controller/controller.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,42 +43,42 @@ class Controller : public QObject
void stopCardEventMonitorThread();

public: // slots
void run();
void run() noexcept;

// Called either directly from run() or from the monitor thread when cards are available.
void onCardsAvailable(const std::vector<electronic_id::ElectronicID::ptr>& availableEids);
void
onCardsAvailable(const std::vector<electronic_id::ElectronicID::ptr>& availableEids) noexcept;

// Called when CommandHandlerRunThread finishes execution.
void onCertificatesLoaded();
void onCertificatesLoaded() noexcept;

// Called either directly from onDialogOK().
void onConfirmCommandHandler(const EidCertificateAndPinInfo& certAndPinInfo) noexcept;

// Called from CommandHandlerConfirm thread.
void onCommandHandlerConfirmCompleted(const QVariantMap& result);
void onCommandHandlerConfirmCompleted(const QVariantMap& result) noexcept;

// Called from the dialog when user chooses to retry errors that have occured in child threads.
void onRetry();
void onRetry() noexcept;

// User events from the dialog.
void onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo);
void onDialogCancel();
void onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo) noexcept;
void onDialogCancel() noexcept;

// Failure handler, reports the error and quits the application.
void onCriticalFailure(const QString& error);
void onCriticalFailure(const QString& error) noexcept;

private:
void startCommandExecution();
void runCommandHandler(std::vector<electronic_id::ElectronicID::ptr> availableEids) noexcept;
void connectRetry(const ControllerChildThread* childThread) const;
void createWindow();
void disposeUI();
void exit();
void waitForChildThreads();
CommandType commandType();
void createWindow() noexcept;
void disposeUI() noexcept;
void exit() noexcept;
void waitForChildThreads() noexcept;
CommandType commandType() const noexcept;

CommandWithArgumentsPtr command;
CommandHandler::ptr commandHandler = nullptr;
CommandHandler::ptr commandHandler;
// As the Qt::WA_DeleteOnClose flag is set, the dialog is deleted automatically.
observer_ptr<WebEidUI> window = nullptr;
QVariantMap _result;
Expand Down