From 67a8fb1fba7f2f4091393bfbf10c83a764841d88 Mon Sep 17 00:00:00 2001
From: Viktor Govako <viktor.govako@gmail.com>
Date: Fri, 20 Aug 2021 11:22:21 +0300
Subject: [PATCH] Fixed possible races in pinger.

Signed-off-by: Viktor Govako <viktor.govako@gmail.com>
---
 storage/map_files_downloader_with_ping.cpp |  7 +++--
 storage/pinger.cpp                         | 30 ++++++++++++++--------
 storage/pinger.hpp                         |  2 +-
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/storage/map_files_downloader_with_ping.cpp b/storage/map_files_downloader_with_ping.cpp
index 612c20975f..4e0bd2060f 100644
--- a/storage/map_files_downloader_with_ping.cpp
+++ b/storage/map_files_downloader_with_ping.cpp
@@ -12,11 +12,10 @@ void MapFilesDownloaderWithPing::GetServersList(ServersListCallback const & call
 {
   ASSERT(callback , ());
 
-  auto urls = LoadServersList();
-
+  auto const urls = LoadServersList();
   CHECK(!urls.empty(), ());
 
-  auto const availableEndpoints = Pinger::ExcludeUnavailableEndpoints(urls);
-  callback(availableEndpoints.empty() ? urls : availableEndpoints);
+  auto const sorted = Pinger::ExcludeUnavailableAndSortEndpoints(urls);
+  callback(sorted.empty() ? urls : sorted);
 }
 }  // namespace storage
diff --git a/storage/pinger.cpp b/storage/pinger.cpp
index 1ade5dad90..1b65aac656 100644
--- a/storage/pinger.cpp
+++ b/storage/pinger.cpp
@@ -18,13 +18,14 @@ using namespace std::chrono;
 namespace
 {
 auto constexpr kTimeoutInSeconds = 4.0;
+int64_t constexpr kInvalidPing = -1;
 
 int64_t DoPing(string const & url)
 {
   if (url.empty())
   {
     ASSERT(false, ("Metaserver returned an empty url."));
-    return -1;
+    return kInvalidPing;
   }
 
   platform::HttpClient request(url);
@@ -40,37 +41,44 @@ int64_t DoPing(string const & url)
     LOG(LWARNING, ("Request to server", url, "failed with code =", request.ErrorCode(), "; redirection =", request.WasRedirected()));
   }
 
-  return -1;
+  return kInvalidPing;
 }
 }  // namespace
 
 namespace storage
 {
 // static
-Pinger::Endpoints Pinger::ExcludeUnavailableEndpoints(Endpoints const & urls)
+Pinger::Endpoints Pinger::ExcludeUnavailableAndSortEndpoints(Endpoints const & urls)
 {
   using base::thread_pool::delayed::ThreadPool;
 
   auto const size = urls.size();
   CHECK_GREATER(size, 0, ());
 
-  map<int64_t, size_t> timeUrls;
+  using EntryT = std::pair<int64_t, size_t>;
+  std::vector<EntryT> timeUrls(size, {kInvalidPing, 0});
   {
-    ThreadPool t(size, ThreadPool::Exit::ExecPending);
+    ThreadPool pool(size, ThreadPool::Exit::ExecPending);
     for (size_t i = 0; i < size; ++i)
     {
-      t.Push([&urls, &timeUrls, i]
+      pool.Push([&urls, &timeUrls, i]
       {
-        auto const pingTime = DoPing(urls[i]);
-        if (pingTime > 0)
-          timeUrls[pingTime] = i;
+        timeUrls[i] = { DoPing(urls[i]), i };
       });
     }
   }
 
+  std::sort(timeUrls.begin(), timeUrls.end(), [](EntryT const & e1, EntryT const & e2)
+  {
+    return e1.first < e2.first;
+  });
+
   Endpoints readyUrls;
-  for (auto const & [_, index] : timeUrls)
-    readyUrls.push_back(urls[index]);
+  for (auto const & [ping, index] : timeUrls)
+  {
+    if (ping >= 0)
+      readyUrls.push_back(urls[index]);
+  }
 
   return readyUrls;
 }
diff --git a/storage/pinger.hpp b/storage/pinger.hpp
index 45e2e243ed..6bc00a2217 100644
--- a/storage/pinger.hpp
+++ b/storage/pinger.hpp
@@ -12,6 +12,6 @@ class Pinger
 public:
   using Endpoints = std::vector<std::string>;
   // Returns list of available endpoints. Works synchronously.
-  static Endpoints ExcludeUnavailableEndpoints(Endpoints const & urls);
+  static Endpoints ExcludeUnavailableAndSortEndpoints(Endpoints const & urls);
 };
 }  // namespace storage