From f3d64fafb692c2aefc83e43d7099482e64684749 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 14 Mar 2018 11:23:12 +0300 Subject: Correct using m_routingSessionMutex for RoutingSession members and review fixes. --- routing/routing_session.cpp | 85 ++++++++++++++++++++++++++++++++++++++++----- routing/routing_session.hpp | 62 ++++++++++++++++++--------------- 2 files changed, 111 insertions(+), 36 deletions(-) (limited to 'routing') diff --git a/routing/routing_session.cpp b/routing/routing_session.cpp index f5806c57e2..9d6b143bd2 100644 --- a/routing/routing_session.cpp +++ b/routing/routing_session.cpp @@ -73,7 +73,7 @@ RoutingSession::RoutingSession() { } -void RoutingSession::Init(TRoutingStatisticsCallback const & routingStatisticsFn, +void RoutingSession::Init(RoutingStatisticsCallback const & routingStatisticsFn, RouterDelegate::TPointCheckCallback const & pointCheckCallback) { threads::MutexGuard guard(m_routingSessionMutex); @@ -98,7 +98,7 @@ void RoutingSession::BuildRoute(Checkpoints const & checkpoints, } void RoutingSession::RebuildRoute(m2::PointD const & startPoint, - TReadyCallback const & readyCallback, uint32_t timeoutSec, + ReadyCallback const & readyCallback, uint32_t timeoutSec, State routeRebuildingState, bool adjustToPrevRoute) { { @@ -196,6 +196,72 @@ void RoutingSession::RebuildRouteOnTrafficUpdate() routing::RoutingSession::State::RouteRebuilding, false /* adjustToPrevRoute */); } +bool RoutingSession::IsActive() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return (m_state != RoutingNotActive); +} + +bool RoutingSession::IsNavigable() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return IsNavigableImpl(); +} + +bool RoutingSession::IsBuilt() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return (IsNavigableImpl() || m_state == RouteNeedRebuild); +} + +bool RoutingSession::IsBuilding() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return (m_state == RouteBuilding || m_state == RouteRebuilding); +} + +bool RoutingSession::IsBuildingOnly() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return m_state == RouteBuilding; +} + +bool RoutingSession::IsRebuildingOnly() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return m_state == RouteRebuilding; +} + +bool RoutingSession::IsNotReady() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return m_state == RouteNotReady; +} + +bool RoutingSession::IsFinished() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return m_state == RouteFinished; +} + +bool RoutingSession::IsNoFollowing() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return m_state == RouteNoFollowing; +} + +bool RoutingSession::IsOnRoute() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return (m_state == OnRoute); +} + +bool RoutingSession::IsFollowing() const +{ + threads::MutexGuard guard(m_routingSessionMutex); + return m_isFollowing; +} + void RoutingSession::Reset() { threads::MutexGuard guard(m_routingSessionMutex); @@ -220,6 +286,7 @@ void RoutingSession::ResetImpl() RoutingSession::State RoutingSession::OnLocationPositionChanged(GpsInfo const & info, Index const & index) { + threads::MutexGuard guard(m_routingSessionMutex); ASSERT(m_state != RoutingNotActive, ()); ASSERT(m_router != nullptr, ()); @@ -228,7 +295,6 @@ RoutingSession::State RoutingSession::OnLocationPositionChanged(GpsInfo const & || m_state == RouteNotReady || m_state == RouteNoFollowing) return m_state; - threads::MutexGuard guard(m_routingSessionMutex); ASSERT(m_route, ()); ASSERT(m_route->IsValid(), ()); @@ -326,7 +392,7 @@ void RoutingSession::GetRouteFollowingInfo(FollowingInfo & info) const return; } - if (!IsNavigable()) + if (!IsNavigableImpl()) { info = FollowingInfo(); FormatDistance(m_route->GetTotalDistanceMeters(), info.m_distToTarget, info.m_targetUnitsSuffix); @@ -431,7 +497,7 @@ void RoutingSession::GenerateTurnNotifications(vector & turnNotification if (!m_routingSettings.m_soundDirection) return; - if (!m_route->IsValid() || !IsNavigable()) + if (!m_route->IsValid() || !IsNavigableImpl()) return; vector turns; @@ -502,6 +568,7 @@ traffic::SpeedGroup RoutingSession::MatchTraffic( bool RoutingSession::DisableFollowMode() { LOG(LINFO, ("Routing disables a following mode. State: ", m_state.load())); + threads::MutexGuard guard(m_routingSessionMutex); if (m_state == RouteNotStarted || m_state == OnRoute) { SetState(RouteNoFollowing); @@ -514,6 +581,7 @@ bool RoutingSession::DisableFollowMode() bool RoutingSession::EnableFollowMode() { LOG(LINFO, ("Routing enables a following mode. State: ", m_state.load())); + threads::MutexGuard guard(m_routingSessionMutex); if (m_state == RouteNotStarted || m_state == OnRoute) { SetState(OnRoute); @@ -528,8 +596,8 @@ void RoutingSession::SetRoutingSettings(RoutingSettings const & routingSettings) m_routingSettings = routingSettings; } -void RoutingSession::SetReadyCallbacks(TReadyCallback const & buildReadyCallback, - TReadyCallback const & rebuildReadyCallback) +void RoutingSession::SetReadyCallbacks(ReadyCallback const & buildReadyCallback, + ReadyCallback const & rebuildReadyCallback) { m_buildReadyCallback = buildReadyCallback; // m_rebuildReadyCallback used from multiple threads but it's the only place we write m_rebuildReadyCallback @@ -542,7 +610,7 @@ void RoutingSession::SetReadyCallbacks(TReadyCallback const & buildReadyCallback m_rebuildReadyCallback = rebuildReadyCallback; } -void RoutingSession::SetProgressCallback(TProgressCallback const & progressCallback) +void RoutingSession::SetProgressCallback(ProgressCallback const & progressCallback) { m_progressCallback = progressCallback; } @@ -554,6 +622,7 @@ void RoutingSession::SetCheckpointCallback(CheckpointCallback const & checkpoint void RoutingSession::SetUserCurrentPosition(m2::PointD const & position) { + threads::MutexGuard guard(m_routingSessionMutex); // All methods which read/write m_userCurrentPosition*, m_userFormerPosition* work in RoutingManager thread m_userFormerPosition = m_userCurrentPosition; m_userFormerPositionValid = m_userCurrentPositionValid; diff --git a/routing/routing_session.hpp b/routing/routing_session.hpp index 2c138633b4..cfa179e91a 100644 --- a/routing/routing_session.hpp +++ b/routing/routing_session.hpp @@ -21,6 +21,7 @@ #include "base/mutex.hpp" #include "std/atomic.hpp" +#include "std/functional.hpp" #include "std/limits.hpp" #include "std/map.hpp" #include "std/shared_ptr.hpp" @@ -75,16 +76,15 @@ public: * RouteFinished -> RouteNotReady // start new route */ - typedef function const &)> TRoutingStatisticsCallback; - - typedef function TReadyCallback; - typedef function TProgressCallback; - typedef function CheckpointCallback; + using RoutingStatisticsCallback = function const &)>; + using ReadyCallback = function; + using ProgressCallback = function; + using CheckpointCallback = function; using RouteCallback = function; RoutingSession(); - void Init(TRoutingStatisticsCallback const & routingStatisticsFn, + void Init(RoutingStatisticsCallback const & routingStatisticsFn, RouterDelegate::TPointCheckCallback const & pointCheckCallback); void SetRouter(unique_ptr && router, unique_ptr && fetcher); @@ -93,24 +93,25 @@ public: /// @param[in] timeoutSec timeout in seconds, if zero then there is no timeout void BuildRoute(Checkpoints const & checkpoints, uint32_t timeoutSec); - void RebuildRoute(m2::PointD const & startPoint, TReadyCallback const & readyCallback, + void RebuildRoute(m2::PointD const & startPoint, ReadyCallback const & readyCallback, uint32_t timeoutSec, State routeRebuildingState, bool adjustToPrevRoute); m2::PointD GetStartPoint() const; m2::PointD GetEndPoint() const; - bool IsActive() const { return (m_state != RoutingNotActive); } - bool IsNavigable() const { return (m_state == RouteNotStarted || m_state == OnRoute || m_state == RouteFinished); } - bool IsBuilt() const { return (IsNavigable() || m_state == RouteNeedRebuild); } + + bool IsActive() const; + bool IsNavigable() const; + bool IsBuilt() const; /// \returns true if a new route is in process of building rebuilding or /// if a route is being rebuilt in case the user left the route, and false otherwise. - bool IsBuilding() const { return (m_state == RouteBuilding || m_state == RouteRebuilding); } - bool IsBuildingOnly() const { return m_state == RouteBuilding; } - bool IsRebuildingOnly() const { return m_state == RouteRebuilding; } - bool IsNotReady() const { return m_state == RouteNotReady; } - bool IsFinished() const { return m_state == RouteFinished; } - bool IsNoFollowing() const { return m_state == RouteNoFollowing; } - bool IsOnRoute() const { return (m_state == OnRoute); } - bool IsFollowing() const { return m_isFollowing; } + bool IsBuilding() const; + bool IsBuildingOnly() const; + bool IsRebuildingOnly() const; + bool IsNotReady() const; + bool IsFinished() const; + bool IsNoFollowing() const; + bool IsOnRoute() const; + bool IsFollowing() const; void Reset(); inline void SetState(State state) { m_state = state; } @@ -148,9 +149,9 @@ public: bool EnableFollowMode(); void SetRoutingSettings(RoutingSettings const & routingSettings); - void SetReadyCallbacks(TReadyCallback const & buildReadyCallback, - TReadyCallback const & rebuildReadyCallback); - void SetProgressCallback(TProgressCallback const & progressCallback); + void SetReadyCallbacks(ReadyCallback const & buildReadyCallback, + ReadyCallback const & rebuildReadyCallback); + void SetProgressCallback(ProgressCallback const & progressCallback); void SetCheckpointCallback(CheckpointCallback const & checkpointCallback); // Sound notifications for turn instructions. @@ -178,10 +179,10 @@ private: struct DoReadyCallback { RoutingSession & m_rs; - TReadyCallback m_callback; + ReadyCallback m_callback; threads::Mutex & m_routeSessionMutexInner; - DoReadyCallback(RoutingSession & rs, TReadyCallback const & cb, + DoReadyCallback(RoutingSession & rs, ReadyCallback const & cb, threads::Mutex & routeSessionMutex) : m_rs(rs), m_callback(cb), m_routeSessionMutexInner(routeSessionMutex) { @@ -190,21 +191,26 @@ private: void operator()(Route & route, IRouter::ResultCode e); }; + // Should be called with locked m_routingSessionMutex. void AssignRoute(Route & route, IRouter::ResultCode e); /// Returns a nearest speed camera record on your way and distance to it. /// Returns kInvalidSpeedCameraDistance if there is no cameras on your way. + // Should be called with locked m_routingSessionMutex. double GetDistanceToCurrentCamM(SpeedCameraRestriction & camera, Index const & index); /// RemoveRoute removes m_route and resets route attributes (m_state, m_lastDistance, m_moveAwayCounter). void RemoveRoute(); void RebuildRouteOnTrafficUpdate(); - // Must be called with locked m_routingSessionMutex + // Should be called with locked m_routingSessionMutex. void ResetImpl(); - + // Should be called with locked m_routingSessionMutex. double GetCompletionPercent() const; + // Should be called with locked m_routingSessionMutex. void PassCheckpoints(); + // Should be called with locked m_routingSessionMutex. + bool IsNavigableImpl() const { return (m_state == RouteNotStarted || m_state == OnRoute || m_state == RouteFinished); } private: unique_ptr m_router; @@ -241,9 +247,9 @@ private: RoutingSettings m_routingSettings; - TReadyCallback m_buildReadyCallback; - TReadyCallback m_rebuildReadyCallback; - TProgressCallback m_progressCallback; + ReadyCallback m_buildReadyCallback; + ReadyCallback m_rebuildReadyCallback; + ProgressCallback m_progressCallback; CheckpointCallback m_checkpointCallback; // Statistics parameters -- cgit v1.2.3