Merge pull request #503 from gardster/routing_rebuild_fix

Routing session tests UB fix.

1 files changed, 52 insertions, 15 deletions

diff --git a/routing/routing_tests/routing_session_test.cpp b/routing/routing_tests/routing_session_test.cpp
index 62ae8c2d0e..a2cc532acd 100644
--- a/routing/routing_tests/routing_session_test.cpp
+++ b/routing/routing_tests/routing_session_test.cpp
@@ -44,6 +44,33 @@ public:
 static vector<m2::PointD> kTestRoute = {{0., 1.}, {0., 2.}, {0., 3.}, {0., 4.}};
 static auto kRouteBuildingMaxDuration = seconds(30);
 
+class TimedSignal
+{
+public:
+  TimedSignal() : m_flag(false) {}
+  void Signal()
+  {
+    lock_guard<mutex> guard(m_waitingMutex);
+    m_flag = true;
+    m_cv.notify_one();
+  }
+
+  bool WaitUntil(steady_clock::time_point const & time)
+  {
+    unique_lock<mutex> lock(m_waitingMutex);
+    m_cv.wait_until(lock, time, [this, &time]
+                    {
+                      return m_flag || steady_clock::now() > time;
+                    });
+    return m_flag;
+  }
+
+private:
+  mutex m_waitingMutex;
+  condition_variable m_cv;
+  bool m_flag;
+};
+
 UNIT_TEST(TestRouteBuilding)
 {
   RoutingSession session;
@@ -51,17 +78,18 @@ UNIT_TEST(TestRouteBuilding)
   vector<m2::PointD> routePoints = kTestRoute;
   Route masterRoute("dummy", routePoints.begin(), routePoints.end());
   size_t counter = 0;
-  timed_mutex routeBuilded;
-  routeBuilded.lock();
+  TimedSignal timedSignal;
   unique_ptr<DummyRouter> router = make_unique<DummyRouter>(masterRoute, DummyRouter::NoError, counter);
   session.SetRouter(move(router), nullptr);
   session.BuildRoute(kTestRoute.front(), kTestRoute.back(),
-                     [&routeBuilded](Route const &, IRouter::ResultCode)
+                     [&timedSignal](Route const &, IRouter::ResultCode)
                      {
-                       routeBuilded.unlock();
+                       timedSignal.Signal();
                      },
                      nullptr, 0);
-  TEST(routeBuilded.try_lock_for(kRouteBuildingMaxDuration), ());
+  // Manual check of the routeBuilded mutex to avoid spurious results.
+  auto const time = steady_clock::now() + kRouteBuildingMaxDuration;
+  TEST(timedSignal.WaitUntil(time), ("Route was not built."));
   TEST_EQUAL(counter, 1, ());
 }
 
@@ -73,18 +101,20 @@ UNIT_TEST(TestRouteRebuilding)
   vector<m2::PointD> routePoints = kTestRoute;
   Route masterRoute("dummy", routePoints.begin(), routePoints.end());
   size_t counter = 0;
-  timed_mutex routeBuilded;
-  auto fn = [&routeBuilded](Route const &, IRouter::ResultCode)
-            {
-              routeBuilded.unlock();
-            };
-  routeBuilded.lock();
   unique_ptr<DummyRouter> router = make_unique<DummyRouter>(masterRoute, DummyRouter::NoError, counter);
   session.SetRouter(move(router), nullptr);
 
   // Go along the route.
-  session.BuildRoute(kTestRoute.front(), kTestRoute.back(), fn, nullptr, 0);
-  TEST(routeBuilded.try_lock_for(kRouteBuildingMaxDuration), ());
+  TimedSignal alongTimedSignal;
+  session.BuildRoute(kTestRoute.front(), kTestRoute.back(),
+                     [&alongTimedSignal](Route const &, IRouter::ResultCode)
+                     {
+                       alongTimedSignal.Signal();
+                     },
+                     nullptr, 0);
+  // Manual check of the routeBuilded mutex to avoid spurious results.
+  auto time = steady_clock::now() + kRouteBuildingMaxDuration;
+  TEST(alongTimedSignal.WaitUntil(time), ("Route was not built."));
 
   location::GpsInfo info;
   info.m_horizontalAccuracy = 0.01;
@@ -102,9 +132,16 @@ UNIT_TEST(TestRouteRebuilding)
   TEST_EQUAL(counter, 1, ());
 
   // Rebuild route and go in opposite direction. So initiate a route rebuilding flag.
+  time = steady_clock::now() + kRouteBuildingMaxDuration;
   counter = 0;
-  session.BuildRoute(kTestRoute.front(), kTestRoute.back(), fn, nullptr, 0);
-  TEST(routeBuilded.try_lock_for(kRouteBuildingMaxDuration), ());
+  TimedSignal oppositeTimedSignal;
+  session.BuildRoute(kTestRoute.front(), kTestRoute.back(),
+                     [&oppositeTimedSignal](Route const &, IRouter::ResultCode)
+                     {
+                       oppositeTimedSignal.Signal();
+                     },
+                     nullptr, 0);
+  TEST(oppositeTimedSignal.WaitUntil(time), ("Route was not built."));
 
   info.m_longitude = 0.;
   info.m_latitude = 1.;