Skip to content

Commit 1da7836

Browse files
metsmametsma
committed
Fix exception handling in Qt signal/slot system
Signed-off-by: Raul Metsma <[email protected]>
1 parent 7defef0 commit 1da7836

File tree

3 files changed

+127
-143
lines changed

3 files changed

+127
-143
lines changed

‎src/controller/commands.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ class CommandType
4040
NONE = -1
4141
};
4242

43-
CommandType() = default;
44-
constexpr CommandType(const CommandTypeEnum _value) : value(_value) {}
43+
CommandType() noexcept = default;
44+
constexpr CommandType(const CommandTypeEnum _value) noexcept : value(_value) {}
4545

4646
constexpr bool operator==(CommandTypeEnum other) const { return value == other; }
4747
constexpr bool operator!=(CommandTypeEnum other) const { return value != other; }

‎src/controller/controller.cpp‎

Lines changed: 109 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ namespace
4242
const QString RESP_TECH_ERROR = QStringLiteral("ERR_WEBEID_NATIVE_FATAL");
4343
const QString RESP_USER_CANCEL = QStringLiteral("ERR_WEBEID_USER_CANCELLED");
4444

45-
QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage)
45+
QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessage) noexcept
4646
{
4747
return {{QStringLiteral("error"),
4848
QVariantMap {
@@ -53,8 +53,8 @@ QVariantMap makeErrorObject(const QString& errorCode, const QString& errorMessag
5353

5454
} // namespace
5555

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

66-
try {
67-
// TODO: cut out stdin mode separate class to avoid bugs in safari mode
68-
if (isInStdinMode) {
69-
// In stdin/stdout mode we first output the version as required by the WebExtension
70-
// and then wait for the actual command.
71-
writeResponseToStdOut(
72-
isInStdinMode,
73-
{{QStringLiteral("version"), QCoreApplication::applicationVersion()}},
74-
"get-version");
75-
76-
command = readCommandFromStdin();
77-
}
78-
79-
REQUIRE_NON_NULL(command)
80-
switch (command->first) {
81-
case CommandType::ABOUT:
82-
WebEidUI::showAboutPage();
83-
return;
84-
case CommandType::QUIT:
85-
// If quit is requested, respond with empty JSON object and quit immediately.
86-
qInfo() << "Quit requested, exiting";
87-
writeResponseToStdOut(true, {}, "quit");
88-
emit quit();
89-
return;
90-
default:
91-
break;
92-
}
93-
94-
commandHandler = getCommandHandler(*command);
95-
96-
startCommandExecution();
97-
98-
} catch (const std::exception& error) {
99-
onCriticalFailure(error.what());
66+
// TODO: cut out stdin mode separate class to avoid bugs in safari mode
67+
if (isInStdinMode) {
68+
// In stdin/stdout mode we first output the version as required by the WebExtension
69+
// and then wait for the actual command.
70+
writeResponseToStdOut(isInStdinMode,
71+
{{QStringLiteral("version"), QCoreApplication::applicationVersion()}},
72+
"get-version");
73+
74+
command = readCommandFromStdin();
75+
}
76+
77+
REQUIRE_NON_NULL(command)
78+
switch (command->first) {
79+
case CommandType::ABOUT:
80+
WebEidUI::showAboutPage();
81+
return;
82+
case CommandType::QUIT:
83+
// If quit is requested, respond with empty JSON object and quit immediately.
84+
qInfo() << "Quit requested, exiting";
85+
writeResponseToStdOut(true, {}, "quit");
86+
emit quit();
87+
return;
88+
default:
89+
break;
10090
}
91+
92+
commandHandler = getCommandHandler(*command);
93+
94+
startCommandExecution();
95+
96+
} catch (const std::exception& error) {
97+
onCriticalFailure(error.what());
10198
}
10299

103100
void Controller::startCommandExecution()
@@ -121,7 +118,7 @@ void Controller::startCommandExecution()
121118
waitForCardThread->start();
122119
}
123120

124-
void Controller::createWindow()
121+
void Controller::createWindow() noexcept
125122
{
126123
window = WebEidUI::createAndShowDialog(commandHandler->commandType());
127124
connect(this, &Controller::statusUpdate, window, &WebEidUI::onSmartCardStatusUpdate);
@@ -135,53 +132,46 @@ void Controller::createWindow()
135132
}
136133

137134
void Controller::onCardsAvailable(
138-
const std::vector<electronic_id::ElectronicID::ptr>& availableEids)
139-
{
140-
try {
141-
REQUIRE_NON_NULL(commandHandler)
142-
REQUIRE_NON_NULL(window)
143-
REQUIRE_NOT_EMPTY_CONTAINS_NON_NULL_PTRS(availableEids)
135+
const std::vector<electronic_id::ElectronicID::ptr>& availableEids) noexcept
136+
try {
137+
REQUIRE_NON_NULL(commandHandler)
138+
REQUIRE_NON_NULL(window)
139+
REQUIRE_NOT_EMPTY_CONTAINS_NON_NULL_PTRS(availableEids)
144140

145-
for (const auto& card : availableEids) {
146-
const auto protocol =
147-
card->smartcard().protocol() == SmartCard::Protocol::T0 ? "T=0" : "T=1";
148-
qInfo() << "Card" << card->name() << "in reader" << card->smartcard().readerName()
149-
<< "using protocol" << protocol;
150-
}
141+
for (const auto& card : availableEids) {
142+
const auto protocol =
143+
card->smartcard().protocol() == SmartCard::Protocol::T0 ? "T=0" : "T=1";
144+
qInfo() << "Card" << card->name() << "in reader" << card->smartcard().readerName()
145+
<< "using protocol" << protocol;
146+
}
151147

152-
window->showWaitingForCardPage(commandHandler->commandType());
148+
window->showWaitingForCardPage(commandHandler->commandType());
153149

154-
commandHandler->connectSignals(window);
150+
commandHandler->connectSignals(window);
155151

156-
runCommandHandler(availableEids);
152+
runCommandHandler(availableEids);
157153

158-
} catch (const std::exception& error) {
159-
onCriticalFailure(error.what());
160-
}
154+
} catch (const std::exception& error) {
155+
onCriticalFailure(error.what());
161156
}
162157

163158
void Controller::runCommandHandler(const std::vector<ElectronicID::ptr>& availableEids)
164159
{
165-
try {
166-
auto* commandHandlerRunThread =
167-
new CommandHandlerRunThread(this, *commandHandler, availableEids);
168-
connectRetry(commandHandlerRunThread);
169-
170-
// When the command handler run thread retrieves certificates successfully, call
171-
// onCertificatesLoaded() that starts card event monitoring while user enters the PIN.
172-
connect(commandHandler.get(), &CommandHandler::singleCertificateReady, this,
173-
&Controller::onCertificatesLoaded);
174-
connect(commandHandler.get(), &CommandHandler::multipleCertificatesReady, this,
175-
&Controller::onCertificatesLoaded);
176-
177-
commandHandlerRunThread->start();
178-
179-
} catch (const std::exception& error) {
180-
onCriticalFailure(error.what());
181-
}
160+
auto* commandHandlerRunThread =
161+
new CommandHandlerRunThread(this, *commandHandler, availableEids);
162+
connectRetry(commandHandlerRunThread);
163+
164+
// When the command handler run thread retrieves certificates successfully, call
165+
// onCertificatesLoaded() that starts card event monitoring while user enters the PIN.
166+
connect(commandHandler.get(), &CommandHandler::singleCertificateReady, this,
167+
&Controller::onCertificatesLoaded);
168+
connect(commandHandler.get(), &CommandHandler::multipleCertificatesReady, this,
169+
&Controller::onCertificatesLoaded);
170+
171+
commandHandlerRunThread->start();
182172
}
183173

184-
void Controller::onCertificatesLoaded()
174+
void Controller::onCertificatesLoaded() noexcept
185175
{
186176
auto* cardEventMonitorThread = new CardEventMonitorThread(this, commandType());
187177
connect(this, &Controller::stopCardEventMonitorThread, cardEventMonitorThread,
@@ -192,7 +182,7 @@ void Controller::onCertificatesLoaded()
192182
cardEventMonitorThread->start();
193183
}
194184

195-
void Controller::disposeUI()
185+
void Controller::disposeUI() noexcept
196186
{
197187
if (window) {
198188
window->disconnect();
@@ -202,60 +192,49 @@ void Controller::disposeUI()
202192
}
203193
}
204194

205-
void Controller::onConfirmCommandHandler(const EidCertificateAndPinInfo& certAndPinInfo)
206-
{
195+
void Controller::onConfirmCommandHandler(const EidCertificateAndPinInfo& certAndPinInfo) noexcept
196+
try {
207197
emit stopCardEventMonitorThread();
208198

209-
try {
210-
auto* commandHandlerConfirmThread =
211-
new CommandHandlerConfirmThread(this, *commandHandler, window, certAndPinInfo);
212-
connect(commandHandlerConfirmThread, &CommandHandlerConfirmThread::completed, this,
213-
&Controller::onCommandHandlerConfirmCompleted);
214-
connectRetry(commandHandlerConfirmThread);
199+
auto* commandHandlerConfirmThread =
200+
new CommandHandlerConfirmThread(this, *commandHandler, window, certAndPinInfo);
201+
connect(commandHandlerConfirmThread, &CommandHandlerConfirmThread::completed, this,
202+
&Controller::onCommandHandlerConfirmCompleted);
203+
connectRetry(commandHandlerConfirmThread);
215204

216-
commandHandlerConfirmThread->start();
205+
commandHandlerConfirmThread->start();
217206

218-
} catch (const std::exception& error) {
219-
onCriticalFailure(error.what());
220-
}
207+
} catch (const std::exception& error) {
208+
onCriticalFailure(error.what());
221209
}
222210

223-
void Controller::onCommandHandlerConfirmCompleted(const QVariantMap& res)
224-
{
225-
REQUIRE_NON_NULL(window)
226-
211+
void Controller::onCommandHandlerConfirmCompleted(const QVariantMap& res) noexcept
212+
try {
227213
qDebug() << "Command completed";
228214

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

232-
try {
233-
_result = res;
234-
writeResponseToStdOut(isInStdinMode, res, commandHandler->commandType());
235-
} catch (const std::exception& error) {
236-
qCritical() << "Command" << std::string(commandType())
237-
<< "fatal error while writing response to stdout:" << error;
238-
}
239-
240-
window->quit();
218+
exit();
219+
} catch (const std::exception& error) {
220+
qCritical() << "Command" << std::string(commandType())
221+
<< "fatal error while writing response to stdout:" << error;
241222
}
242223

243-
void Controller::onRetry()
244-
{
245-
try {
246-
// Dispose the UI, it will be re-created during next execution.
247-
disposeUI();
248-
// Command handler signals are still connected, disconnect them so that they can be
249-
// reconnected during next execution.
250-
commandHandler->disconnect();
251-
// Before restarting, wait until child threads finish.
252-
waitForChildThreads();
253-
254-
startCommandExecution();
255-
256-
} catch (const std::exception& error) {
257-
onCriticalFailure(error.what());
258-
}
224+
void Controller::onRetry() noexcept
225+
try {
226+
// Dispose the UI, it will be re-created during next execution.
227+
disposeUI();
228+
// Command handler signals are still connected, disconnect them so that they can be
229+
// reconnected during next execution.
230+
commandHandler->disconnect();
231+
// Before restarting, wait until child threads finish.
232+
waitForChildThreads();
233+
234+
startCommandExecution();
235+
236+
} catch (const std::exception& error) {
237+
onCriticalFailure(error.what());
259238
}
260239

261240
void Controller::connectRetry(const ControllerChildThread* childThread)
@@ -267,7 +246,7 @@ void Controller::connectRetry(const ControllerChildThread* childThread)
267246
connect(childThread, &ControllerChildThread::cancel, this, &Controller::onDialogCancel);
268247
}
269248

270-
void Controller::onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo)
249+
void Controller::onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo) noexcept
271250
{
272251
if (commandHandler) {
273252
onConfirmCommandHandler(certAndPinInfo);
@@ -277,16 +256,18 @@ void Controller::onDialogOK(const EidCertificateAndPinInfo& certAndPinInfo)
277256
}
278257
}
279258

280-
void Controller::onDialogCancel()
281-
{
259+
void Controller::onDialogCancel() noexcept
260+
try {
282261
qDebug() << "User cancelled";
283262
_result = makeErrorObject(RESP_USER_CANCEL, QStringLiteral("User cancelled"));
284263
writeResponseToStdOut(isInStdinMode, _result, commandType());
285264
exit();
265+
} catch (const std::exception& e) {
266+
onCriticalFailure(e.what());
286267
}
287268

288-
void Controller::onCriticalFailure(const QString& error)
289-
{
269+
void Controller::onCriticalFailure(const QString& error) noexcept
270+
try {
290271
emit stopCardEventMonitorThread();
291272
qCritical() << "Exiting due to command" << std::string(commandType())
292273
<< "fatal error:" << error;
@@ -304,16 +285,18 @@ void Controller::onCriticalFailure(const QString& error)
304285
writeResponseToStdOut(isInStdinMode, _result, commandType());
305286
}
306287
exit();
288+
} catch (const std::exception& e) {
289+
qCritical() << "Failed to write stdout" << e.what();
307290
}
308291

309-
void Controller::exit()
292+
void Controller::exit() noexcept
310293
{
311294
disposeUI();
312295
waitForChildThreads();
313296
emit quit();
314297
}
315298

316-
void Controller::waitForChildThreads()
299+
void Controller::waitForChildThreads() noexcept
317300
{
318301
for (auto* thread : findChildren<QThread*>()) {
319302
qDebug() << "Interrupting thread" << uintptr_t(thread);
@@ -324,7 +307,7 @@ void Controller::waitForChildThreads()
324307
}
325308
}
326309

327-
CommandType Controller::commandType()
310+
CommandType Controller::commandType() const noexcept
328311
{
329312
return commandHandler ? commandHandler->commandType() : CommandType(CommandType::INSERT_CARD);
330313
}

0 commit comments

Comments
 (0)