From 265af2d299010967bbe372cad32b89b150ce9f04 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Wed, 22 Feb 2023 19:39:24 +0100 Subject: [PATCH] fix(GODT-2389): close bridge on exception and add max termination wait time. --- internal/frontend/bridge-gui/bridge-gui/main.cpp | 16 +++++++--------- .../bridgepp/bridgepp/Worker/Overseer.cpp | 3 ++- .../bridgepp/bridgepp/Worker/Overseer.h | 3 +++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/frontend/bridge-gui/bridge-gui/main.cpp b/internal/frontend/bridge-gui/bridge-gui/main.cpp index 90b1ebe2..1b0338bb 100644 --- a/internal/frontend/bridge-gui/bridge-gui/main.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/main.cpp @@ -117,8 +117,9 @@ QQmlComponent *createRootQmlComponent(QQmlApplicationEngine &engine) { rootComponent->loadUrl(QUrl(qrcQmlDir + "/Bridge.qml")); if (rootComponent->status() != QQmlComponent::Status::Ready) { - app().log().error(rootComponent->errorString()); - throw Exception("Could not load QML component"); + QString const &err =rootComponent->errorString(); + app().log().error(err); + throw Exception("Could not load QML component", err); } return rootComponent; } @@ -255,12 +256,8 @@ void closeBridgeApp() { app().grpc().quit(); // this will cause the grpc service and the bridge app to close. UPOverseer &overseer = app().bridgeOverseer(); - if (!overseer) { // The app was run in 'attach' mode and attached to an existing instance of Bridge. We're not monitoring it. - return; - } - - while (!overseer->isFinished()) { - QThread::msleep(20); + if (overseer) { // A null overseer means the app was run in 'attach' mode. We're not monitoring it. + overseer->wait(Overseer::maxTerminationWaitTimeMs); } } @@ -387,7 +384,7 @@ int main(int argc, char *argv[]) { QObject::disconnect(connection); app().grpc().stopEventStreamReader(); - if (!app().backend().waitForEventStreamReaderToFinish(5000)) { + if (!app().backend().waitForEventStreamReaderToFinish(Overseer::maxTerminationWaitTimeMs)) { log.warn("Event stream reader took too long to finish."); } @@ -413,6 +410,7 @@ int main(int argc, char *argv[]) { errStream << "reportID: " << QByteArray(uuid.bytes, 16).toHex() << " Captured exception :" << e.qwhat() << "\n"; if (hasDetails) errStream << "\nDetails:\n" << e.details() << "\n"; + closeBridgeApp(); return EXIT_FAILURE; } } diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.cpp index ed4553bc..8a5da3b4 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.cpp @@ -84,7 +84,8 @@ void Overseer::releaseWorker() { if (thread_) { if (!thread_->isFinished()) { thread_->quit(); - thread_->wait(); + if (!thread_->wait(maxTerminationWaitTimeMs)) + thread_->terminate(); } thread_->deleteLater(); thread_ = nullptr; diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.h index 26943899..e4bdd47a 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Worker/Overseer.h @@ -46,6 +46,9 @@ public slots: void startWorker(bool autorelease) const; ///< Run the worker. void releaseWorker(); ///< Delete the worker and its thread. +public: // static data members + static qint64 const maxTerminationWaitTimeMs { 10000 }; ///< The maximum wait time for the termination of a thread + public: // data members. QThread *thread_ { nullptr }; ///< The thread. Worker *worker_ { nullptr }; ///< The worker.