From e2b6336ef792ef95aaa3e35be056cb6c6ec212f6 Mon Sep 17 00:00:00 2001 From: Oliver Eichler Date: Fri, 11 Aug 2023 08:34:11 +0200 Subject: [PATCH 1/2] [QMS-624] Catch exception calcRoute properly and mark methods as noexecpt(false) --- changelog.txt | 4 +++- src/qmapshack/gis/rte/router/CRouterBRouter.cpp | 7 ++++--- src/qmapshack/gis/rte/router/CRouterBRouter.h | 5 +++-- src/qmapshack/gis/rte/router/CRouterOptimization.cpp | 12 ++++++++++-- src/qmapshack/gis/rte/router/CRouterRoutino.cpp | 3 ++- src/qmapshack/gis/rte/router/CRouterRoutino.h | 2 +- src/qmapshack/gis/rte/router/CRouterSetup.cpp | 2 +- src/qmapshack/gis/rte/router/CRouterSetup.h | 2 +- src/qmapshack/gis/rte/router/IRouter.h | 3 ++- 9 files changed, 27 insertions(+), 13 deletions(-) diff --git a/changelog.txt b/changelog.txt index 706a8e026..9ecb3bb09 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,7 @@ +V1.XX.X [QMS-622] Update BRouter setup (install from github) -[QMS-623] remove use of QTimer in brouter startup error detection +[QMS-623] Remove use of QTimer in brouter startup error detection +[QMS-624] Fix crashes while using BRouter [QMS-630] BRouter on-the-fly routing cannot be canceled V1.17.0 diff --git a/src/qmapshack/gis/rte/router/CRouterBRouter.cpp b/src/qmapshack/gis/rte/router/CRouterBRouter.cpp index c9c6b5d6f..1546e9f81 100644 --- a/src/qmapshack/gis/rte/router/CRouterBRouter.cpp +++ b/src/qmapshack/gis/rte/router/CRouterBRouter.cpp @@ -195,7 +195,8 @@ QString CRouterBRouter::getOptions() { void CRouterBRouter::routerSelected() { getBRouterVersion(); } bool CRouterBRouter::hasFastRouting() { - return setup->installMode == CRouterBRouterSetup::eModeLocal && setup->isLocalBRouterValid && checkFastRecalc->isChecked(); + return setup->installMode == CRouterBRouterSetup::eModeLocal && setup->isLocalBRouterValid && + checkFastRecalc->isChecked(); } QNetworkRequest CRouterBRouter::getRequest(const QVector& routePoints, const QList& nogos) const { @@ -282,7 +283,7 @@ QNetworkRequest CRouterBRouter::getRequest(const QVector& routePoints, return QNetworkRequest(url); } -int CRouterBRouter::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs) { +int CRouterBRouter::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs) noexcept(false) { if (!hasFastRouting()) { return -1; } @@ -296,7 +297,7 @@ int CRouterBRouter::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& c } int CRouterBRouter::synchronousRequest(const QVector& points, const QList& nogos, QPolygonF& coords, - qreal* costs = nullptr) { + qreal* costs = nullptr) noexcept(false) { if (!mutex.tryLock()) { // skip further on-the-fly-requests as long a previous request is still running return -1; diff --git a/src/qmapshack/gis/rte/router/CRouterBRouter.h b/src/qmapshack/gis/rte/router/CRouterBRouter.h index 6ba3daf4d..4ccfb50b7 100644 --- a/src/qmapshack/gis/rte/router/CRouterBRouter.h +++ b/src/qmapshack/gis/rte/router/CRouterBRouter.h @@ -43,7 +43,8 @@ class CRouterBRouter : public IRouter, private Ui::IRouterBRouter { static CRouterBRouter& self() { return *pSelf; } void calcRoute(const IGisItem::key_t& key) override; - int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs = nullptr) override; + int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, + qreal* costs = nullptr) noexcept(false) override; bool hasFastRouting() override; QString getOptions() override; void routerSelected() override; @@ -70,7 +71,7 @@ class CRouterBRouter : public IRouter, private Ui::IRouterBRouter { bool isMinimumVersion(int major, int minor, int patch) const; void updateBRouterStatus() const; int synchronousRequest(const QVector& points, const QList& nogos, QPolygonF& coords, - qreal* costs); + qreal* costs) noexcept(false); QNetworkRequest getRequest(const QVector& routePoints, const QList& nogos) const; QUrl getServiceUrl() const; diff --git a/src/qmapshack/gis/rte/router/CRouterOptimization.cpp b/src/qmapshack/gis/rte/router/CRouterOptimization.cpp index 74497c4cc..25686741c 100644 --- a/src/qmapshack/gis/rte/router/CRouterOptimization.cpp +++ b/src/qmapshack/gis/rte/router/CRouterOptimization.cpp @@ -18,6 +18,8 @@ #include "CRouterOptimization.h" +#include + #include "gis/GeoMath.h" #include "gis/rte/router/CRouterSetup.h" #include "helpers/CProgressDialog.h" @@ -237,10 +239,16 @@ const CRouterOptimization::routing_cache_item_t* CRouterOptimization::getRoute(c if (!routingCache[start_key].contains(end_key)) { routing_cache_item_t cacheItem; - int response = CRouterSetup::self().calcRoute(start, end, cacheItem.route, &cacheItem.costs); - if (response < 0) { + try { + int response = CRouterSetup::self().calcRoute(start, end, cacheItem.route, &cacheItem.costs); + if (response < 0) { + return nullptr; + } + } catch (const QString& msg) { + qWarning() << msg; return nullptr; } + routingCache[start_key][end_key] = cacheItem; qreal airToCostFactor = cacheItem.costs / GPS_Math_DistanceQuick(start.x(), start.y(), end.x(), end.y()); diff --git a/src/qmapshack/gis/rte/router/CRouterRoutino.cpp b/src/qmapshack/gis/rte/router/CRouterRoutino.cpp index 805f25eb7..27156b02e 100644 --- a/src/qmapshack/gis/rte/router/CRouterRoutino.cpp +++ b/src/qmapshack/gis/rte/router/CRouterRoutino.cpp @@ -402,7 +402,8 @@ void CRouterRoutino::calcRoute(const IGisItem::key_t& key) { CCanvas::triggerCompleteUpdate(CCanvas::eRedrawGis); } -int CRouterRoutino::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs = nullptr) { +int CRouterRoutino::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, + qreal* costs = nullptr) noexcept(false) { if (!mutex.tryLock()) { return -1; } diff --git a/src/qmapshack/gis/rte/router/CRouterRoutino.h b/src/qmapshack/gis/rte/router/CRouterRoutino.h index f7bd64cc8..76c1c4f71 100644 --- a/src/qmapshack/gis/rte/router/CRouterRoutino.h +++ b/src/qmapshack/gis/rte/router/CRouterRoutino.h @@ -35,7 +35,7 @@ class CRouterRoutino : public IRouter, private Ui::IRouterRoutino { static CRouterRoutino& self() { return *pSelf; } void calcRoute(const IGisItem::key_t& key) override; - int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs) override; + int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs) noexcept(false) override; bool hasFastRouting() override; diff --git a/src/qmapshack/gis/rte/router/CRouterSetup.cpp b/src/qmapshack/gis/rte/router/CRouterSetup.cpp index 31696f463..a7ba118fc 100644 --- a/src/qmapshack/gis/rte/router/CRouterSetup.cpp +++ b/src/qmapshack/gis/rte/router/CRouterSetup.cpp @@ -75,7 +75,7 @@ void CRouterSetup::calcRoute(const IGisItem::key_t& key) { } } -int CRouterSetup::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs) { +int CRouterSetup::calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs) noexcept(false) { IRouter* router = dynamic_cast(stackedWidget->currentWidget()); if (router) { return router->calcRoute(p1, p2, coords, costs); diff --git a/src/qmapshack/gis/rte/router/CRouterSetup.h b/src/qmapshack/gis/rte/router/CRouterSetup.h index bf4cb400e..52c725331 100644 --- a/src/qmapshack/gis/rte/router/CRouterSetup.h +++ b/src/qmapshack/gis/rte/router/CRouterSetup.h @@ -31,7 +31,7 @@ class CRouterSetup : public QWidget, private Ui::IRouterSetup { virtual ~CRouterSetup(); void calcRoute(const IGisItem::key_t& key); - int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs = nullptr); + int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs = nullptr) noexcept(false); QString getOptions(); bool hasFastRouting(); diff --git a/src/qmapshack/gis/rte/router/IRouter.h b/src/qmapshack/gis/rte/router/IRouter.h index 724cedc33..75e6a71e4 100644 --- a/src/qmapshack/gis/rte/router/IRouter.h +++ b/src/qmapshack/gis/rte/router/IRouter.h @@ -30,7 +30,8 @@ class IRouter : public QWidget { virtual ~IRouter(); virtual void calcRoute(const IGisItem::key_t& key) = 0; - virtual int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, qreal* costs = nullptr) = 0; + virtual int calcRoute(const QPointF& p1, const QPointF& p2, QPolygonF& coords, + qreal* costs = nullptr) noexcept(false) = 0; virtual bool hasFastRouting() { return fastRouting; } virtual QString getOptions() = 0; From 3a6d083590ad622651c40e9187023fe06818852a Mon Sep 17 00:00:00 2001 From: Norbert Truchsess Date: Fri, 11 Aug 2023 21:50:48 +0200 Subject: [PATCH 2/2] QMS-624 fix BRouter synchronous request return value in case of error --- src/qmapshack/gis/rte/router/CRouterBRouter.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qmapshack/gis/rte/router/CRouterBRouter.cpp b/src/qmapshack/gis/rte/router/CRouterBRouter.cpp index 1546e9f81..9a56053fc 100644 --- a/src/qmapshack/gis/rte/router/CRouterBRouter.cpp +++ b/src/qmapshack/gis/rte/router/CRouterBRouter.cpp @@ -381,12 +381,12 @@ int CRouterBRouter::synchronousRequest(const QVector& points, const QLi } } } catch (const QString& msg) { - coords.clear(); + reply->deleteLater(); + mutex.unlock(); if (!msg.isEmpty()) { - reply->deleteLater(); - mutex.unlock(); throw tr("Bad response from server: %1").arg(msg); } + return -1; } reply->deleteLater();