diff options
author | 2021-03-24 15:04:31 +0000 | |
---|---|---|
committer | 2021-03-24 15:04:31 +0000 | |
commit | 16828074ec56fe8a2393bfcbf341cbdbaddd07cf (patch) | |
tree | 835cc41a9f24918684d1517902908b21f4d696bf | |
parent | 0c1e189278d023dc2649335cb2afc583cef2fb9f (diff) | |
parent | 9acc9acea544605c85b27a9e2b157fec6766e983 (diff) |
Merge "[incfs] Fix the mount state callbacks processing" into sc-dev
-rw-r--r-- | services/incremental/IncrementalService.cpp | 43 | ||||
-rw-r--r-- | services/incremental/IncrementalService.h | 5 | ||||
-rw-r--r-- | services/incremental/test/IncrementalServiceTest.cpp | 32 |
3 files changed, 44 insertions, 36 deletions
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp index 217b621e54ab..9bb2f041556a 100644 --- a/services/incremental/IncrementalService.cpp +++ b/services/incremental/IncrementalService.cpp @@ -2272,7 +2272,7 @@ void IncrementalService::addIfsStateCallback(StorageId storageId, IfsStateCallba mIfsStateCallbacks[storageId].emplace_back(std::move(callback)); } if (wasEmpty) { - addTimedJob(*mTimedQueue, kMaxStorageId, Constants::progressUpdateInterval, + addTimedJob(*mTimedQueue, kAllStoragesId, Constants::progressUpdateInterval, [this]() { processIfsStateCallbacks(); }); } } @@ -2288,29 +2288,36 @@ void IncrementalService::processIfsStateCallbacks() { } IfsStateCallbacks::iterator it; if (storageId == kInvalidStorageId) { - // First entry, initialize the it. + // First entry, initialize the |it|. it = mIfsStateCallbacks.begin(); } else { - // Subsequent entries, update the storageId, and shift to the new one. - it = mIfsStateCallbacks.find(storageId); + // Subsequent entries, update the |storageId|, and shift to the new one (not that + // it guarantees much about updated items, but at least the loop will finish). + it = mIfsStateCallbacks.lower_bound(storageId); if (it == mIfsStateCallbacks.end()) { - // Was removed while processing, too bad. + // Nothing else left, too bad. break; } - - auto& callbacks = it->second; - if (callbacks.empty()) { - std::swap(callbacks, local); + if (it->first != storageId) { + local.clear(); // Was removed during processing, forget the old callbacks. } else { - callbacks.insert(callbacks.end(), local.begin(), local.end()); - } - if (callbacks.empty()) { - it = mIfsStateCallbacks.erase(it); - if (mIfsStateCallbacks.empty()) { - return; + // Put the 'surviving' callbacks back into the map and advance the position. + auto& callbacks = it->second; + if (callbacks.empty()) { + std::swap(callbacks, local); + } else { + callbacks.insert(callbacks.end(), std::move_iterator(local.begin()), + std::move_iterator(local.end())); + local.clear(); + } + if (callbacks.empty()) { + it = mIfsStateCallbacks.erase(it); + if (mIfsStateCallbacks.empty()) { + return; + } + } else { + ++it; } - } else { - ++it; } } @@ -2330,7 +2337,7 @@ void IncrementalService::processIfsStateCallbacks() { processIfsStateCallbacks(storageId, local); } - addTimedJob(*mTimedQueue, kMaxStorageId, Constants::progressUpdateInterval, + addTimedJob(*mTimedQueue, kAllStoragesId, Constants::progressUpdateInterval, [this]() { processIfsStateCallbacks(); }); } diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h index 105d40c1822a..a697305457f8 100644 --- a/services/incremental/IncrementalService.h +++ b/services/incremental/IncrementalService.h @@ -95,7 +95,8 @@ public: #pragma GCC diagnostic pop static constexpr StorageId kInvalidStorageId = -1; - static constexpr StorageId kMaxStorageId = std::numeric_limits<int>::max(); + static constexpr StorageId kMaxStorageId = std::numeric_limits<int>::max() - 1; + static constexpr StorageId kAllStoragesId = kMaxStorageId + 1; static constexpr BootClockTsUs kMaxBootClockTsUs = std::numeric_limits<BootClockTsUs>::max(); @@ -472,7 +473,7 @@ private: std::mutex mCallbacksLock; std::unordered_map<std::string, sp<AppOpsListener>> mCallbackRegistered; - using IfsStateCallbacks = std::unordered_map<StorageId, std::vector<IfsStateCallback>>; + using IfsStateCallbacks = std::map<StorageId, std::vector<IfsStateCallback>>; std::mutex mIfsStateCallbacksLock; IfsStateCallbacks mIfsStateCallbacks; diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp index e6d487255eff..8ba7c8686cba 100644 --- a/services/incremental/test/IncrementalServiceTest.cpp +++ b/services/incremental/test/IncrementalServiceTest.cpp @@ -1735,10 +1735,10 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDone) { ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); // IfsState callback present. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); auto callback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Not loaded yet. EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) @@ -1751,10 +1751,10 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDone) { ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); // Still present. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); callback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Fully loaded. EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)).WillOnce(Return(incfs::LoadingState::Full)); @@ -1797,10 +1797,10 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDoneWithReadlogs) { ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); // IfsState callback present. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); auto callback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Not loaded yet. EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) @@ -1813,10 +1813,10 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDoneWithReadlogs) { ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); // Still present. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); callback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Fully loaded. EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) @@ -1832,10 +1832,10 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDoneWithReadlogs) { ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); // Still present. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); callback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Disable readlogs and expect the unbind. EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(1); @@ -2007,10 +2007,10 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { { // Timed callback present -> 0 progress. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1)); const auto timedCallback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Call it again. timedCallback(); @@ -2018,10 +2018,10 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { { // Still present -> some progress. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1)); const auto timedCallback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Fully loaded but readlogs collection enabled. ASSERT_GE(mDataLoader->setStorageParams(true), 0); @@ -2032,10 +2032,10 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { { // Still present -> fully loaded + readlogs. - ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kAllStoragesId, mTimedQueue->mId); ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1)); const auto timedCallback = mTimedQueue->mWhat; - mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + mTimedQueue->clearJob(IncrementalService::kAllStoragesId); // Now disable readlogs. ASSERT_GE(mDataLoader->setStorageParams(false), 0); |