From 0c575913f6aa9b3e51d27af56c712ff11fe2f08f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beat=20K=C3=BCng?= Date: Fri, 10 Nov 2017 17:07:58 +0100 Subject: [PATCH] AirMapManager: add LifetimeChecker class to prevent callbacks from accessing invalid memory --- src/MissionManager/AirMapManager.cc | 54 +++++++++++++++++++++-------- src/MissionManager/AirMapManager.h | 25 ++++++++++--- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/MissionManager/AirMapManager.cc b/src/MissionManager/AirMapManager.cc index 0faba7ff9..c82995a72 100644 --- a/src/MissionManager/AirMapManager.cc +++ b/src/MissionManager/AirMapManager.cc @@ -155,9 +155,11 @@ void AirMapRestrictionManager::setROI(const QGeoCoordinate& center, double radiu params.geometry = Geometry::point(center.latitude(), center.longitude()); params.buffer = radiusMeters; params.full = true; + std::weak_ptr isAlive(_instance); _shared.client()->airspaces().search(params, - [this](const Airspaces::Search::Result& result) { + [this, isAlive](const Airspaces::Search::Result& result) { + if (!isAlive.lock()) return; if (_state != State::RetrieveItems) return; if (result) { @@ -282,11 +284,15 @@ void AirMapFlightManager::createFlight(const QList& missionItems) // need to get the pilot id before uploading the flight qCDebug(AirMapManagerLog) << "Getting pilot ID"; _state = State::GetPilotID; - _shared.doRequestWithLogin([this](const QString& login_token) { + std::weak_ptr isAlive(_instance); + _shared.doRequestWithLogin([this, isAlive](const QString& login_token) { + if (!isAlive.lock()) return; + Pilots::Authenticated::Parameters params; params.authorization = login_token.toStdString(); - _shared.client()->pilots().authenticated(params, [this](const Pilots::Authenticated::Result& result) { + _shared.client()->pilots().authenticated(params, [this, isAlive](const Pilots::Authenticated::Result& result) { + if (!isAlive.lock()) return; if (_state != State::GetPilotID) return; if (result) { @@ -326,7 +332,9 @@ void AirMapFlightManager::_endFirstFlight() params.pilot_id = _pilotID.toStdString(); params.end_after = Clock::universal_time() - Hours{1}; - _shared.client()->flights().search(params, [this](const Flights::Search::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flights().search(params, [this, isAlive](const Flights::Search::Result& result) { + if (!isAlive.lock()) return; if (_state != State::EndFirstFlight) return; if (result && result.value().size() > 0) { @@ -336,7 +344,8 @@ void AirMapFlightManager::_endFirstFlight() Flights::EndFlight::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = result.value()[0].id; // pick the first flight (TODO: match the vehicle id) - _shared.client()->flights().end_flight(params, [this](const Flights::EndFlight::Result& result) { + _shared.client()->flights().end_flight(params, [this, isAlive](const Flights::EndFlight::Result& result) { + if (!isAlive.lock()) return; if (_state != State::EndFirstFlight) return; if (!result) { @@ -375,8 +384,10 @@ void AirMapFlightManager::_uploadFlight() qCDebug(AirMapManagerLog) << "uploading flight"; _state = State::FlightUpload; - _shared.doRequestWithLogin([this](const QString& login_token) { + std::weak_ptr isAlive(_instance); + _shared.doRequestWithLogin([this, isAlive](const QString& login_token) { + if (!isAlive.lock()) return; if (_state != State::FlightUpload) return; FlightPlans::Create::Parameters params; @@ -407,7 +418,8 @@ void AirMapFlightManager::_uploadFlight() params.authorization = login_token.toStdString(); _flight.coords.clear(); - _shared.client()->flight_plans().create_by_polygon(params, [this](const FlightPlans::Create::Result& result) { + _shared.client()->flight_plans().create_by_polygon(params, [this, isAlive](const FlightPlans::Create::Result& result) { + if (!isAlive.lock()) return; if (_state != State::FlightUpload) return; if (result) { @@ -432,7 +444,9 @@ void AirMapFlightManager::_checkForValidBriefing() FlightPlans::RenderBriefing::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = _pendingFlightPlan.toStdString(); - _shared.client()->flight_plans().render_briefing(params, [this](const FlightPlans::RenderBriefing::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flight_plans().render_briefing(params, [this, isAlive](const FlightPlans::RenderBriefing::Result& result) { + if (!isAlive.lock()) return; if (_state != State::FlightBrief) return; if (result) { @@ -464,7 +478,9 @@ void AirMapFlightManager::_submitPendingFlightPlan() FlightPlans::Submit::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = _pendingFlightPlan.toStdString(); - _shared.client()->flight_plans().submit(params, [this](const FlightPlans::Submit::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flight_plans().submit(params, [this, isAlive](const FlightPlans::Submit::Result& result) { + if (!isAlive.lock()) return; if (_state != State::FlightSubmit) return; if (result) { @@ -488,7 +504,9 @@ void AirMapFlightManager::_pollBriefing() FlightPlans::RenderBriefing::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = _pendingFlightPlan.toStdString(); - _shared.client()->flight_plans().render_briefing(params, [this](const FlightPlans::RenderBriefing::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flight_plans().render_briefing(params, [this, isAlive](const FlightPlans::RenderBriefing::Result& result) { + if (!isAlive.lock()) return; if (_state != State::FlightPolling) return; if (result) { @@ -567,7 +585,9 @@ void AirMapFlightManager::_endFlight(const QString& flightID) Flights::EndFlight::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = flightID.toStdString(); - _shared.client()->flights().end_flight(params, [this](const Flights::EndFlight::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flights().end_flight(params, [this, isAlive](const Flights::EndFlight::Result& result) { + if (!isAlive.lock()) return; if (_state != State::FlightEnd) return; _state = State::Idle; @@ -675,7 +695,9 @@ void AirMapTelemetry::startTelemetryStream(const QString& flightID) Flights::StartFlightCommunications::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = _flightID.toStdString(); - _shared.client()->flights().start_flight_communications(params, [this](const Flights::StartFlightCommunications::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flights().start_flight_communications(params, [this, isAlive](const Flights::StartFlightCommunications::Result& result) { + if (!isAlive.lock()) return; if (_state != State::StartCommunication) return; if (result) { @@ -704,8 +726,10 @@ void AirMapTelemetry::stopTelemetryStream() Flights::EndFlightCommunications::Parameters params; params.authorization = _shared.loginToken().toStdString(); params.id = _flightID.toStdString(); - _shared.client()->flights().end_flight_communications(params, [this](const Flights::EndFlightCommunications::Result& result) { + std::weak_ptr isAlive(_instance); + _shared.client()->flights().end_flight_communications(params, [this, isAlive](const Flights::EndFlightCommunications::Result& result) { Q_UNUSED(result); + if (!isAlive.lock()) return; if (_state != State::EndCommunication) return; _key = ""; @@ -721,7 +745,9 @@ AirMapTrafficMonitor::~AirMapTrafficMonitor() void AirMapTrafficMonitor::startConnection(const QString& flightID) { _flightID = flightID; - auto handler = [this](const Traffic::Monitor::Result& result) { + std::weak_ptr isAlive(_instance); + auto handler = [this, isAlive](const Traffic::Monitor::Result& result) { + if (!isAlive.lock()) return; if (result) { _monitor = result.value(); _subscriber = std::make_shared( diff --git a/src/MissionManager/AirMapManager.h b/src/MissionManager/AirMapManager.h index bd76c3aa1..02363352b 100644 --- a/src/MissionManager/AirMapManager.h +++ b/src/MissionManager/AirMapManager.h @@ -32,6 +32,23 @@ Q_DECLARE_LOGGING_CATEGORY(AirMapManagerLog) + +/** + * @class LifetimeChecker + * Base class which helps to check if an object instance still exists. + * A subclass can take a weak pointer from _instance and then check if the object was deleted. + * This is used in callbacks that access 'this', but the instance might already be deleted (e.g. vehicle disconnect). + */ +class LifetimeChecker +{ +public: + LifetimeChecker() : _instance(this, [](void*){}) { } + virtual ~LifetimeChecker() = default; + +protected: + std::shared_ptr _instance; +}; + /** * @class AirMapSharedState * contains state & settings that need to be shared (such as login) @@ -96,7 +113,7 @@ private: /// class to download polygons from AirMap -class AirMapRestrictionManager : public AirspaceRestrictionProvider +class AirMapRestrictionManager : public AirspaceRestrictionProvider, public LifetimeChecker { Q_OBJECT public: @@ -122,7 +139,7 @@ private: /// class to upload a flight -class AirMapFlightManager : public QObject +class AirMapFlightManager : public QObject, public LifetimeChecker { Q_OBJECT public: @@ -203,7 +220,7 @@ private: }; /// class to send telemetry data to AirMap -class AirMapTelemetry : public QObject +class AirMapTelemetry : public QObject, public LifetimeChecker { Q_OBJECT public: @@ -247,7 +264,7 @@ private: }; -class AirMapTrafficMonitor : public QObject +class AirMapTrafficMonitor : public QObject, public LifetimeChecker { Q_OBJECT public: -- 2.22.0