From a94b181f2fc86c50b177fb710ab36809b2eaded0 Mon Sep 17 00:00:00 2001 From: VladiMihaylenko Date: Wed, 21 Feb 2018 11:19:00 +0300 Subject: [PATCH] Hold file and network threads as unique_ptrs --- map/framework.cpp | 2 +- map/framework.hpp | 4 ++ platform/platform.cpp | 15 ++++- platform/platform.hpp | 61 +++++++++++-------- storage/storage.cpp | 11 ++-- .../storage_downloading_tests.cpp | 1 + .../storage_update_tests.cpp | 2 +- 7 files changed, 64 insertions(+), 32 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index f0019b4439..bf89c65ebd 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -484,7 +484,7 @@ Framework::Framework(FrameworkParams const & params) Framework::~Framework() { - GetPlatform().ShutdownThreads(); + m_threadRunner.reset(); osm::Editor & editor = osm::Editor::Instance(); diff --git a/map/framework.hpp b/map/framework.hpp index 17d6f4223a..ba8ca0e0d8 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -145,6 +145,10 @@ class Framework : public SearchAPI::Delegate, public RoutingManager::Delegate } m_fixedPos; #endif + +private: + // Must be first member in Framework. + unique_ptr m_threadRunner = make_unique(); protected: using TDrapeFunction = function; diff --git a/platform/platform.cpp b/platform/platform.cpp index 86b3e9c2c4..2f180fbd9f 100644 --- a/platform/platform.cpp +++ b/platform/platform.cpp @@ -271,8 +271,19 @@ unsigned Platform::CpuCores() const void Platform::ShutdownThreads() { - m_networkThread.ShutdownAndJoin(); - m_fileThread.ShutdownAndJoin(); + ASSERT(m_networkThread && m_fileThread, ()); + m_networkThread->ShutdownAndJoin(); + m_fileThread->ShutdownAndJoin(); + + m_networkThread.reset(); + m_fileThread.reset(); +} + +void Platform::RunThreads() +{ + ASSERT(!m_networkThread && !m_fileThread, ()); + m_networkThread = make_unique(); + m_fileThread = make_unique(); } string DebugPrint(Platform::EError err) diff --git a/platform/platform.hpp b/platform/platform.hpp index 887c929d45..03b9bc4faa 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -30,9 +30,21 @@ namespace platform class LocalCountryFile; } +class Platform; + +extern Platform & GetPlatform(); + class Platform { public: + friend struct ThreadRunner; + + struct ThreadRunner + { + ThreadRunner() { GetPlatform().RunThreads(); } + ~ThreadRunner() { GetPlatform().ShutdownThreads(); } + }; + enum EError { ERR_OK = 0, @@ -111,8 +123,8 @@ protected: unique_ptr m_guiThread; - base::WorkerThread m_networkThread; - base::WorkerThread m_fileThread; + unique_ptr m_networkThread; + unique_ptr m_fileThread; public: Platform(); @@ -263,50 +275,51 @@ public: template void RunTask(Thread thread, Task && task) { + ASSERT(m_networkThread && m_fileThread, ()); switch (thread) { - case Thread::File: - m_fileThread.Push(forward(task)); - break; - case Thread::Network: - m_networkThread.Push(forward(task)); - break; - case Thread::Gui: - RunOnGuiThread(forward(task)); - break; + case Thread::File: + m_fileThread->Push(forward(task)); + break; + case Thread::Network: + m_networkThread->Push(forward(task)); + break; + case Thread::Gui: + RunOnGuiThread(forward(task)); + break; } } template void RunDelayedTask(Thread thread, base::WorkerThread::Duration const & delay, Task && task) { + ASSERT(m_networkThread && m_fileThread, ()); switch (thread) { - case Thread::File: - m_fileThread.PushDelayed(delay, forward(task)); - break; - case Thread::Network: - m_networkThread.PushDelayed(delay, forward(task)); - break; - case Thread::Gui: - CHECK(false, ("Delayed tasks for gui thread are not supported yet")); - break; + case Thread::File: + m_fileThread->PushDelayed(delay, forward(task)); + break; + case Thread::Network: + m_networkThread->PushDelayed(delay, forward(task)); + break; + case Thread::Gui: + CHECK(false, ("Delayed tasks for gui thread are not supported yet")); + break; } } - void ShutdownThreads(); - // Use this method for testing purposes only. void SetGuiThread(unique_ptr guiThread); private: + void RunThreads(); + void ShutdownThreads(); + void RunOnGuiThread(base::TaskLoop::Task && task); void RunOnGuiThread(base::TaskLoop::Task const & task); void GetSystemFontNames(FilesList & res) const; }; -extern Platform & GetPlatform(); - string DebugPrint(Platform::EError err); string DebugPrint(Platform::ChargingStatus status); diff --git a/storage/storage.cpp b/storage/storage.cpp index d0c7d33a79..8d0d02134e 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -108,7 +108,6 @@ Storage::Storage(string const & pathToCountriesFile /* = COUNTRIES_FILE */, SetLocale(languages::GetCurrentTwine()); LoadCountriesFile(pathToCountriesFile, m_dataDir); CalMaxMwmSizeBytes(); - LoadServerListForSession(); } Storage::Storage(string const & referenceCountriesTxtJsonForTesting, @@ -122,7 +121,6 @@ Storage::Storage(string const & referenceCountriesTxtJsonForTesting, LoadCountriesFromBuffer(referenceCountriesTxtJsonForTesting, m_countries, m_affiliations); CHECK_LESS_OR_EQUAL(0, m_currentVersion, ("Can't load test countries file")); CalMaxMwmSizeBytes(); - LoadServerListForTesting(); } void Storage::Init(TUpdateCallback const & didDownload, TDeleteCallback const & willDelete) @@ -642,10 +640,15 @@ void Storage::DownloadNextFile(QueuedCountry const & country) return; } - if (m_sessionServerList) + if (m_sessionServerList || !m_downloadingUrlsForTesting.empty()) + { DoDownload(); + } else + { + LoadServerListForSession(); SetDeferDownloading(); + } } void Storage::DeleteFromDownloader(TCountryId const & countryId) @@ -786,7 +789,7 @@ void Storage::ReportProgressForHierarchy(TCountryId const & countryId, void Storage::DoDownload() { ASSERT_THREAD_CHECKER(m_threadChecker, ()); - CHECK(m_sessionServerList, ()); + CHECK(m_sessionServerList || !m_downloadingUrlsForTesting.empty(), ()); // Queue can be empty because countries were deleted from queue. if (m_queue.empty()) diff --git a/storage/storage_integration_tests/storage_downloading_tests.cpp b/storage/storage_integration_tests/storage_downloading_tests.cpp index 7809e933f2..6dc668b21b 100644 --- a/storage/storage_integration_tests/storage_downloading_tests.cpp +++ b/storage/storage_integration_tests/storage_downloading_tests.cpp @@ -72,6 +72,7 @@ UNIT_TEST(SmallMwms_ReDownloadExistedMWMIgnored_Test) UNIT_TEST(SmallMwms_InterruptDownloadResumeDownload_Test) { + auto runner = make_unique(); WritableDirChanger writableDirChanger(kMapTestDir); // Start download but interrupt it diff --git a/storage/storage_integration_tests/storage_update_tests.cpp b/storage/storage_integration_tests/storage_update_tests.cpp index c250d9f219..87ef9f4881 100644 --- a/storage/storage_integration_tests/storage_update_tests.cpp +++ b/storage/storage_integration_tests/storage_update_tests.cpp @@ -61,7 +61,7 @@ bool DownloadFile(string const & url, string GetCountriesTxtWebUrl(string const version) { - return kTestWebServer + "/direct/" + version + "/" + kCountriesTxtFile; + return kTestWebServer + "direct/" + version + "/" + kCountriesTxtFile; } string GetCountriesTxtFilePath()