diff options
| -rw-r--r-- | services/incremental/IncrementalService.cpp | 344 | ||||
| -rw-r--r-- | services/incremental/IncrementalService.h | 32 | ||||
| -rw-r--r-- | services/incremental/test/IncrementalServiceTest.cpp | 204 |
3 files changed, 437 insertions, 143 deletions
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp index a88f2b43b909..932e99783c83 100644 --- a/services/incremental/IncrementalService.cpp +++ b/services/incremental/IncrementalService.cpp @@ -377,6 +377,7 @@ void IncrementalService::onDump(int fd) { dprintf(fd, "Mounts (%d): {\n", int(mMounts.size())); for (auto&& [id, ifs] : mMounts) { + std::unique_lock ll(ifs->lock); const IncFsMount& mnt = *ifs; dprintf(fd, " [%d]: {\n", id); if (id != mnt.mountId) { @@ -422,6 +423,9 @@ void IncrementalService::onDump(int fd) { } bool IncrementalService::needStartDataLoaderLocked(IncFsMount& ifs) { + if (!ifs.dataLoaderStub) { + return false; + } if (ifs.dataLoaderStub->isSystemDataLoader()) { return true; } @@ -439,6 +443,8 @@ void IncrementalService::onSystemReady() { std::lock_guard l(mLock); mounts.reserve(mMounts.size()); for (auto&& [id, ifs] : mMounts) { + std::unique_lock ll(ifs->lock); + if (ifs->mountId != id) { continue; } @@ -456,7 +462,10 @@ void IncrementalService::onSystemReady() { std::thread([this, mounts = std::move(mounts)]() { mJni->initializeForCurrentThread(); for (auto&& ifs : mounts) { - ifs->dataLoaderStub->requestStart(); + std::unique_lock l(ifs->lock); + if (ifs->dataLoaderStub) { + ifs->dataLoaderStub->requestStart(); + } } }).detach(); } @@ -671,23 +680,36 @@ bool IncrementalService::startLoading(StorageId storageId, setUidReadTimeouts(storageId, std::move(perUidReadTimeouts)); } + IfsMountPtr ifs; + DataLoaderStubPtr dataLoaderStub; + // Re-initialize DataLoader. - std::unique_lock l(mLock); - const auto ifs = getIfsLocked(storageId); - if (!ifs) { - return false; + { + ifs = getIfs(storageId); + if (!ifs) { + return false; + } + + std::unique_lock l(ifs->lock); + dataLoaderStub = std::exchange(ifs->dataLoaderStub, nullptr); } - if (ifs->dataLoaderStub) { - ifs->dataLoaderStub->cleanupResources(); - ifs->dataLoaderStub = {}; + + if (dataLoaderStub) { + dataLoaderStub->cleanupResources(); + dataLoaderStub = {}; } - l.unlock(); - // DataLoader. - auto dataLoaderStub = - prepareDataLoader(*ifs, std::move(dataLoaderParams), std::move(statusListener), - healthCheckParams, std::move(healthListener)); - CHECK(dataLoaderStub); + { + std::unique_lock l(ifs->lock); + if (ifs->dataLoaderStub) { + LOG(INFO) << "Skipped data loader stub creation because it already exists"; + return false; + } + prepareDataLoaderLocked(*ifs, std::move(dataLoaderParams), std::move(statusListener), + healthCheckParams, std::move(healthListener)); + CHECK(ifs->dataLoaderStub); + dataLoaderStub = ifs->dataLoaderStub; + } if (dataLoaderStub->isSystemDataLoader()) { // Readlogs from system dataloader (adb) can always be collected. @@ -705,13 +727,14 @@ bool IncrementalService::startLoading(StorageId storageId, << storageId; return; } + std::unique_lock l(ifs->lock); if (ifs->startLoadingTs != startLoadingTs) { LOG(INFO) << "Can't disable the readlogs, timestamp mismatch (new " "installation?): " << storageId; return; } - setStorageParams(*ifs, storageId, /*enableReadLogs=*/false); + disableReadLogsLocked(*ifs); }); } @@ -733,17 +756,17 @@ StorageId IncrementalService::findStorageId(std::string_view path) const { } void IncrementalService::disallowReadLogs(StorageId storageId) { - std::unique_lock l(mLock); - const auto ifs = getIfsLocked(storageId); + const auto ifs = getIfs(storageId); if (!ifs) { LOG(ERROR) << "disallowReadLogs failed, invalid storageId: " << storageId; return; } + + std::unique_lock l(ifs->lock); if (!ifs->readLogsAllowed()) { return; } ifs->disallowReadLogs(); - l.unlock(); const auto metadata = constants().readLogsDisabledMarkerName; if (auto err = mIncFs->makeFile(ifs->control, @@ -755,7 +778,7 @@ void IncrementalService::disallowReadLogs(StorageId storageId) { return; } - setStorageParams(storageId, /*enableReadLogs=*/false); + disableReadLogsLocked(*ifs); } int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLogs) { @@ -764,61 +787,66 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog LOG(ERROR) << "setStorageParams failed, invalid storageId: " << storageId; return -EINVAL; } - return setStorageParams(*ifs, storageId, enableReadLogs); -} -int IncrementalService::setStorageParams(IncFsMount& ifs, StorageId storageId, - bool enableReadLogs) { - const auto& params = ifs.dataLoaderStub->params(); - if (enableReadLogs) { - if (!ifs.readLogsAllowed()) { - LOG(ERROR) << "setStorageParams failed, readlogs disallowed for storageId: " - << storageId; - return -EPERM; - } + std::unique_lock l(ifs->lock); + if (!enableReadLogs) { + return disableReadLogsLocked(*ifs); + } - // Check loader usage stats permission and apop. - if (auto status = mAppOpsManager->checkPermission(kLoaderUsageStats, kOpUsage, - params.packageName.c_str()); - !status.isOk()) { - LOG(ERROR) << " Permission: " << kLoaderUsageStats - << " check failed: " << status.toString8(); - return fromBinderStatus(status); - } + if (!ifs->readLogsAllowed()) { + LOG(ERROR) << "enableReadLogs failed, readlogs disallowed for storageId: " << storageId; + return -EPERM; + } - // Check multiuser permission. - if (auto status = mAppOpsManager->checkPermission(kInteractAcrossUsers, nullptr, - params.packageName.c_str()); - !status.isOk()) { - LOG(ERROR) << " Permission: " << kInteractAcrossUsers - << " check failed: " << status.toString8(); - return fromBinderStatus(status); - } + if (!ifs->dataLoaderStub) { + // This should never happen - only DL can call enableReadLogs. + LOG(ERROR) << "enableReadLogs failed: invalid state"; + return -EPERM; + } - // Check installation time. - const auto now = mClock->now(); - const auto startLoadingTs = ifs.startLoadingTs; - if (startLoadingTs <= now && now - startLoadingTs > Constants::readLogsMaxInterval) { - LOG(ERROR) << "setStorageParams failed, readlogs can't be enabled at this time, " - "storageId: " - << storageId; - return -EPERM; - } + // Check installation time. + const auto now = mClock->now(); + const auto startLoadingTs = ifs->startLoadingTs; + if (startLoadingTs <= now && now - startLoadingTs > Constants::readLogsMaxInterval) { + LOG(ERROR) << "enableReadLogs failed, readlogs can't be enabled at this time, storageId: " + << storageId; + return -EPERM; } - if (auto status = applyStorageParams(ifs, enableReadLogs); !status.isOk()) { - LOG(ERROR) << "applyStorageParams failed: " << status.toString8(); + const auto& packageName = ifs->dataLoaderStub->params().packageName; + + // Check loader usage stats permission and apop. + if (auto status = + mAppOpsManager->checkPermission(kLoaderUsageStats, kOpUsage, packageName.c_str()); + !status.isOk()) { + LOG(ERROR) << " Permission: " << kLoaderUsageStats + << " check failed: " << status.toString8(); return fromBinderStatus(status); } - if (enableReadLogs) { - registerAppOpsCallback(params.packageName); + // Check multiuser permission. + if (auto status = + mAppOpsManager->checkPermission(kInteractAcrossUsers, nullptr, packageName.c_str()); + !status.isOk()) { + LOG(ERROR) << " Permission: " << kInteractAcrossUsers + << " check failed: " << status.toString8(); + return fromBinderStatus(status); + } + + if (auto status = applyStorageParamsLocked(*ifs, /*enableReadLogs=*/true); status != 0) { + return status; } + registerAppOpsCallback(packageName); + return 0; } -binder::Status IncrementalService::applyStorageParams(IncFsMount& ifs, bool enableReadLogs) { +int IncrementalService::disableReadLogsLocked(IncFsMount& ifs) { + return applyStorageParamsLocked(ifs, /*enableReadLogs=*/false); +} + +int IncrementalService::applyStorageParamsLocked(IncFsMount& ifs, bool enableReadLogs) { os::incremental::IncrementalFileSystemControlParcel control; control.cmd.reset(dup(ifs.control.cmd())); control.pendingReads.reset(dup(ifs.control.pendingReads())); @@ -832,8 +860,10 @@ binder::Status IncrementalService::applyStorageParams(IncFsMount& ifs, bool enab if (status.isOk()) { // Store enabled state. ifs.setReadLogsEnabled(enableReadLogs); + } else { + LOG(ERROR) << "applyStorageParams failed: " << status.toString8(); } - return status; + return status.isOk() ? 0 : fromBinderStatus(status); } void IncrementalService::deleteStorage(StorageId storageId) { @@ -1224,9 +1254,14 @@ void IncrementalService::setUidReadTimeouts(StorageId storage, return; } - const auto timeout = std::chrono::duration_cast<milliseconds>(maxPendingTimeUs) - - Constants::perUidTimeoutOffset; - updateUidReadTimeouts(storage, Clock::now() + timeout); + const auto timeout = Clock::now() + maxPendingTimeUs - Constants::perUidTimeoutOffset; + addIfsStateCallback(storage, [this, timeout](StorageId storageId, IfsState state) -> bool { + if (checkUidReadTimeouts(storageId, state, timeout)) { + return true; + } + clearUidReadTimeouts(storageId); + return false; + }); } void IncrementalService::clearUidReadTimeouts(StorageId storage) { @@ -1234,39 +1269,32 @@ void IncrementalService::clearUidReadTimeouts(StorageId storage) { if (!ifs) { return; } - mIncFs->setUidReadTimeouts(ifs->control, {}); } -void IncrementalService::updateUidReadTimeouts(StorageId storage, Clock::time_point timeLimit) { - // Reached maximum timeout. +bool IncrementalService::checkUidReadTimeouts(StorageId storage, IfsState state, + Clock::time_point timeLimit) { if (Clock::now() >= timeLimit) { - return clearUidReadTimeouts(storage); + // Reached maximum timeout. + return false; } - - // Still loading? - const auto state = isMountFullyLoaded(storage); - if (int(state) < 0) { + if (state.error) { // Something is wrong, abort. - return clearUidReadTimeouts(storage); + return false; } - if (state == incfs::LoadingState::Full) { - // Fully loaded, check readLogs collection. - const auto ifs = getIfs(storage); - if (!ifs->readLogsEnabled()) { - return clearUidReadTimeouts(storage); - } + // Still loading? + if (state.fullyLoaded && !state.readLogsEnabled) { + return false; } const auto timeLeft = timeLimit - Clock::now(); if (timeLeft < Constants::progressUpdateInterval) { // Don't bother. - return clearUidReadTimeouts(storage); + return false; } - addTimedJob(*mTimedQueue, storage, Constants::progressUpdateInterval, - [this, storage, timeLimit]() { updateUidReadTimeouts(storage, timeLimit); }); + return true; } std::unordered_set<std::string_view> IncrementalService::adoptMountedInstances() { @@ -1533,7 +1561,7 @@ bool IncrementalService::mountExistingImage(std::string_view root) { dataLoaderParams.arguments = loader.arguments(); } - prepareDataLoader(*ifs, std::move(dataLoaderParams)); + prepareDataLoaderLocked(*ifs, std::move(dataLoaderParams)); CHECK(ifs->dataLoaderStub); std::vector<std::pair<std::string, metadata::BindPoint>> bindPoints; @@ -1615,24 +1643,10 @@ void IncrementalService::runCmdLooper() { } } -IncrementalService::DataLoaderStubPtr IncrementalService::prepareDataLoader( - IncFsMount& ifs, DataLoaderParamsParcel&& params, DataLoaderStatusListener&& statusListener, - const StorageHealthCheckParams& healthCheckParams, StorageHealthListener&& healthListener) { - std::unique_lock l(ifs.lock); - prepareDataLoaderLocked(ifs, std::move(params), std::move(statusListener), healthCheckParams, - std::move(healthListener)); - return ifs.dataLoaderStub; -} - void IncrementalService::prepareDataLoaderLocked(IncFsMount& ifs, DataLoaderParamsParcel&& params, DataLoaderStatusListener&& statusListener, const StorageHealthCheckParams& healthCheckParams, StorageHealthListener&& healthListener) { - if (ifs.dataLoaderStub) { - LOG(INFO) << "Skipped data loader preparation because it already exists"; - return; - } - FileSystemControlParcel fsControlParcel; fsControlParcel.incremental = std::make_optional<IncrementalFileSystemControlParcel>(); fsControlParcel.incremental->cmd.reset(dup(ifs.control.cmd())); @@ -1647,6 +1661,29 @@ void IncrementalService::prepareDataLoaderLocked(IncFsMount& ifs, DataLoaderPara new DataLoaderStub(*this, ifs.mountId, std::move(params), std::move(fsControlParcel), std::move(statusListener), healthCheckParams, std::move(healthListener), path::join(ifs.root, constants().mount)); + + addIfsStateCallback(ifs.mountId, [this](StorageId storageId, IfsState state) -> bool { + if (!state.fullyLoaded || state.readLogsEnabled) { + return true; + } + + DataLoaderStubPtr dataLoaderStub; + { + const auto ifs = getIfs(storageId); + if (!ifs) { + return false; + } + + std::unique_lock l(ifs->lock); + dataLoaderStub = std::exchange(ifs->dataLoaderStub, nullptr); + } + + if (dataLoaderStub) { + dataLoaderStub->cleanupResources(); + } + + return false; + }); } template <class Duration> @@ -2070,11 +2107,11 @@ bool IncrementalService::registerStorageHealthListener( StorageHealthListener healthListener) { DataLoaderStubPtr dataLoaderStub; { - std::unique_lock l(mLock); - const auto& ifs = getIfsLocked(storage); + const auto& ifs = getIfs(storage); if (!ifs) { return false; } + std::unique_lock l(ifs->lock); dataLoaderStub = ifs->dataLoaderStub; if (!dataLoaderStub) { return false; @@ -2160,13 +2197,16 @@ void IncrementalService::onAppOpChanged(const std::string& packageName) { std::lock_guard l(mLock); affected.reserve(mMounts.size()); for (auto&& [id, ifs] : mMounts) { - if (ifs->mountId == id && ifs->dataLoaderStub->params().packageName == packageName) { + std::unique_lock ll(ifs->lock); + + if (ifs->mountId == id && ifs->dataLoaderStub && + ifs->dataLoaderStub->params().packageName == packageName) { affected.push_back(ifs); } } } for (auto&& ifs : affected) { - applyStorageParams(*ifs, false); + applyStorageParamsLocked(*ifs, /*enableReadLogs=*/false); } } @@ -2187,6 +2227,101 @@ bool IncrementalService::removeTimedJobs(TimedQueueWrapper& timedQueue, MountId return true; } +void IncrementalService::addIfsStateCallback(StorageId storageId, IfsStateCallback callback) { + bool wasEmpty; + { + std::lock_guard l(mIfsStateCallbacksLock); + wasEmpty = mIfsStateCallbacks.empty(); + mIfsStateCallbacks[storageId].emplace_back(std::move(callback)); + } + if (wasEmpty) { + addTimedJob(*mTimedQueue, kMaxStorageId, Constants::progressUpdateInterval, + [this]() { processIfsStateCallbacks(); }); + } +} + +void IncrementalService::processIfsStateCallbacks() { + StorageId storageId = kInvalidStorageId; + std::vector<IfsStateCallback> local; + while (true) { + { + std::lock_guard l(mIfsStateCallbacksLock); + if (mIfsStateCallbacks.empty()) { + return; + } + IfsStateCallbacks::iterator it; + if (storageId == kInvalidStorageId) { + // First entry, initialize the it. + it = mIfsStateCallbacks.begin(); + } else { + // Subsequent entries, update the storageId, and shift to the new one. + it = mIfsStateCallbacks.find(storageId); + if (it == mIfsStateCallbacks.end()) { + // Was removed while processing, too bad. + break; + } + + auto& callbacks = it->second; + if (callbacks.empty()) { + std::swap(callbacks, local); + } else { + callbacks.insert(callbacks.end(), local.begin(), local.end()); + } + if (callbacks.empty()) { + it = mIfsStateCallbacks.erase(it); + if (mIfsStateCallbacks.empty()) { + return; + } + } else { + ++it; + } + } + + if (it == mIfsStateCallbacks.end()) { + break; + } + + storageId = it->first; + auto& callbacks = it->second; + if (callbacks.empty()) { + // Invalid case, one extra lookup should be ok. + continue; + } + std::swap(callbacks, local); + } + + processIfsStateCallbacks(storageId, local); + } + + addTimedJob(*mTimedQueue, kMaxStorageId, Constants::progressUpdateInterval, + [this]() { processIfsStateCallbacks(); }); +} + +void IncrementalService::processIfsStateCallbacks(StorageId storageId, + std::vector<IfsStateCallback>& callbacks) { + const auto state = isMountFullyLoaded(storageId); + IfsState storageState = {}; + storageState.error = int(state) < 0; + storageState.fullyLoaded = state == incfs::LoadingState::Full; + if (storageState.fullyLoaded) { + const auto ifs = getIfs(storageId); + storageState.readLogsEnabled = ifs && ifs->readLogsEnabled(); + } + + for (auto cur = callbacks.begin(); cur != callbacks.end();) { + if ((*cur)(storageId, storageState)) { + ++cur; + } else { + cur = callbacks.erase(cur); + } + } +} + +void IncrementalService::removeIfsStateCallbacks(StorageId storageId) { + std::lock_guard l(mIfsStateCallbacksLock); + mIfsStateCallbacks.erase(storageId); +} + void IncrementalService::getMetrics(StorageId storageId, android::os::PersistableBundle* result) { const auto duration = getMillsSinceOldestPendingRead(storageId); if (duration >= 0) { @@ -2197,12 +2332,12 @@ void IncrementalService::getMetrics(StorageId storageId, android::os::Persistabl } long IncrementalService::getMillsSinceOldestPendingRead(StorageId storageId) { - std::unique_lock l(mLock); - const auto ifs = getIfsLocked(storageId); + const auto ifs = getIfs(storageId); if (!ifs) { LOG(ERROR) << "getMillsSinceOldestPendingRead failed, invalid storageId: " << storageId; return -EINVAL; } + std::unique_lock l(ifs->lock); if (!ifs->dataLoaderStub) { LOG(ERROR) << "getMillsSinceOldestPendingRead failed, no data loader: " << storageId; return -EINVAL; @@ -2248,6 +2383,7 @@ void IncrementalService::DataLoaderStub::cleanupResources() { resetHealthControl(); mService.removeTimedJobs(*mService.mTimedQueue, mId); } + mService.removeIfsStateCallbacks(mId); requestDestroy(); @@ -2758,7 +2894,7 @@ void IncrementalService::DataLoaderStub::registerForPendingReads() { mService.mLooper->addFd( pendingReadsFd, android::Looper::POLL_CALLBACK, android::Looper::EVENT_INPUT, [](int, int, void* data) -> int { - auto&& self = (DataLoaderStub*)data; + auto self = (DataLoaderStub*)data; self->updateHealthStatus(/*baseline=*/true); return 0; }, diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h index 4a5db062e3c5..a8f32dec824e 100644 --- a/services/incremental/IncrementalService.h +++ b/services/incremental/IncrementalService.h @@ -74,6 +74,17 @@ using StorageLoadingProgressListener = ::android::sp<IStorageLoadingProgressList using PerUidReadTimeouts = ::android::os::incremental::PerUidReadTimeouts; +struct IfsState { + // If mount is fully loaded. + bool fullyLoaded = false; + // If read logs are enabled on this mount. Populated only if fullyLoaded == true. + bool readLogsEnabled = false; + // If there was an error fetching any of the above. + bool error = false; +}; +// Returns true if wants to be called again. +using IfsStateCallback = std::function<bool(StorageId, IfsState)>; + class IncrementalService final { public: explicit IncrementalService(ServiceManagerWrapper&& sm, std::string_view rootDir); @@ -366,7 +377,7 @@ private: void setUidReadTimeouts(StorageId storage, std::vector<PerUidReadTimeouts>&& perUidReadTimeouts); void clearUidReadTimeouts(StorageId storage); - void updateUidReadTimeouts(StorageId storage, Clock::time_point timeLimit); + bool checkUidReadTimeouts(StorageId storage, IfsState state, Clock::time_point timeLimit); std::unordered_set<std::string_view> adoptMountedInstances(); void mountExistingImages(const std::unordered_set<std::string_view>& mountedRootNames); @@ -387,11 +398,6 @@ private: bool needStartDataLoaderLocked(IncFsMount& ifs); - DataLoaderStubPtr prepareDataLoader(IncFsMount& ifs, - content::pm::DataLoaderParamsParcel&& params, - DataLoaderStatusListener&& statusListener = {}, - const StorageHealthCheckParams& healthCheckParams = {}, - StorageHealthListener&& healthListener = {}); void prepareDataLoaderLocked(IncFsMount& ifs, content::pm::DataLoaderParamsParcel&& params, DataLoaderStatusListener&& statusListener = {}, const StorageHealthCheckParams& healthCheckParams = {}, @@ -410,8 +416,8 @@ private: std::string_view path) const; int makeDirs(const IncFsMount& ifs, StorageId storageId, std::string_view path, int mode); - int setStorageParams(IncFsMount& ifs, StorageId storageId, bool enableReadLogs); - binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs); + int disableReadLogsLocked(IncFsMount& ifs); + int applyStorageParamsLocked(IncFsMount& ifs, bool enableReadLogs); LoadingProgress getLoadingProgressFromPath(const IncFsMount& ifs, std::string_view path) const; @@ -431,6 +437,12 @@ private: bool addTimedJob(TimedQueueWrapper& timedQueue, MountId id, Milliseconds after, Job what); bool removeTimedJobs(TimedQueueWrapper& timedQueue, MountId id); + + void addIfsStateCallback(StorageId storageId, IfsStateCallback callback); + void removeIfsStateCallbacks(StorageId storageId); + void processIfsStateCallbacks(); + void processIfsStateCallbacks(StorageId storageId, std::vector<IfsStateCallback>& callbacks); + bool updateLoadingProgress(int32_t storageId, StorageLoadingProgressListener&& progressListener); long getMillsSinceOldestPendingRead(StorageId storage); @@ -456,6 +468,10 @@ private: std::mutex mCallbacksLock; std::unordered_map<std::string, sp<AppOpsListener>> mCallbackRegistered; + using IfsStateCallbacks = std::unordered_map<StorageId, std::vector<IfsStateCallback>>; + std::mutex mIfsStateCallbacksLock; + IfsStateCallbacks mIfsStateCallbacks; + std::atomic_bool mSystemReady = false; StorageId mNextId = 0; diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp index 54bc95dc213b..de8822dbf105 100644 --- a/services/incremental/test/IncrementalServiceTest.cpp +++ b/services/incremental/test/IncrementalServiceTest.cpp @@ -140,9 +140,7 @@ public: const content::pm::FileSystemControlParcel& control, const sp<content::pm::IDataLoaderStatusListener>& listener) { createOkNoStatus(id, params, control, listener); - if (mListener) { - mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_CREATED); - } + reportStatus(id); return binder::Status::ok(); } binder::Status createOkNoStatus(int32_t id, const content::pm::DataLoaderParamsParcel& params, @@ -150,33 +148,26 @@ public: const sp<content::pm::IDataLoaderStatusListener>& listener) { mServiceConnector = control.service; mListener = listener; + mStatus = IDataLoaderStatusListener::DATA_LOADER_CREATED; return binder::Status::ok(); } binder::Status startOk(int32_t id) { - if (mListener) { - mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_STARTED); - } + setAndReportStatus(id, IDataLoaderStatusListener::DATA_LOADER_STARTED); return binder::Status::ok(); } binder::Status stopOk(int32_t id) { - if (mListener) { - mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_STOPPED); - } + setAndReportStatus(id, IDataLoaderStatusListener::DATA_LOADER_STOPPED); return binder::Status::ok(); } binder::Status destroyOk(int32_t id) { - if (mListener) { - mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_DESTROYED); - } + setAndReportStatus(id, IDataLoaderStatusListener::DATA_LOADER_DESTROYED); mListener = nullptr; return binder::Status::ok(); } binder::Status prepareImageOk(int32_t id, const ::std::vector<content::pm::InstallationFileParcel>&, const ::std::vector<::std::string>&) { - if (mListener) { - mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_IMAGE_READY); - } + setAndReportStatus(id, IDataLoaderStatusListener::DATA_LOADER_IMAGE_READY); return binder::Status::ok(); } binder::Status storageError(int32_t id) { @@ -197,10 +188,22 @@ public: EXPECT_TRUE(mServiceConnector->setStorageParams(enableReadLogs, &result).isOk()); return result; } + int status() const { return mStatus; } private: + void setAndReportStatus(int id, int status) { + mStatus = status; + reportStatus(id); + } + void reportStatus(int id) { + if (mListener) { + mListener->onStatusChanged(id, mStatus); + } + } + sp<IIncrementalServiceConnector> mServiceConnector; sp<IDataLoaderStatusListener> mListener; + int mStatus = IDataLoaderStatusListener::DATA_LOADER_DESTROYED; }; class MockDataLoaderManager : public DataLoaderManagerWrapper { @@ -915,7 +918,7 @@ TEST_F(IncrementalServiceTest, testDataLoaderOnRestart) { EXPECT_CALL(*mDataLoader, start(_)).Times(6); EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); - EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(3); + EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(4); TemporaryDir tempDir; int storageId = mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel, @@ -1126,7 +1129,7 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnhealthyStorage) { EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); EXPECT_CALL(*mLooper, addFd(MockIncFs::kPendingReadsFd, _, _, _, _)).Times(2); EXPECT_CALL(*mLooper, removeFd(MockIncFs::kPendingReadsFd)).Times(2); - EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(5); + EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(6); sp<NiceMock<MockStorageHealthListener>> listener{new NiceMock<MockStorageHealthListener>}; NiceMock<MockStorageHealthListener>* listenerMock = listener.get(); @@ -1314,7 +1317,7 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndTimedOut) { EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1); // Not expecting callback removal. EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); - EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(1); + EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2); TemporaryDir tempDir; int storageId = mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel, @@ -1355,7 +1358,7 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNoTimedOutForSy EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1); // Not expecting callback removal. EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); - EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(0); + EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2); // System data loader. mDataLoaderParcel.packageName = "android"; TemporaryDir tempDir; @@ -1366,9 +1369,9 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNoTimedOutForSy ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {}, {}, {})); - // No readlogs callback. - ASSERT_EQ(mTimedQueue->mAfter, 0ms); - ASSERT_EQ(mTimedQueue->mWhat, nullptr); + // IfsState callback. + auto callback = mTimedQueue->mWhat; + mTimedQueue->clearJob(storageId); ASSERT_GE(mDataLoader->setStorageParams(true), 0); // Now advance clock for 1hr. @@ -1376,6 +1379,8 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNoTimedOutForSy ASSERT_GE(mDataLoader->setStorageParams(true), 0); // Now advance clock for 2hrs. mClock->advance(readLogsMaxInterval); + // IfsStorage callback should not affect anything. + callback(); ASSERT_EQ(mDataLoader->setStorageParams(true), 0); } @@ -1394,7 +1399,7 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNewInstall) { EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1); // Not expecting callback removal. EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0); - EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2); + EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(4); TemporaryDir tempDir; int storageId = mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel, @@ -1685,6 +1690,144 @@ TEST_F(IncrementalServiceTest, testRegisterLoadingProgressListenerFailsToGetProg mIncrementalService->registerLoadingProgressListener(storageId, listener); } +TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDone) { + mFs->hasFiles(); + + const auto stateUpdateInterval = 1s; + + EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _)).Times(1); + // No unbinding just yet. + EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(0); + EXPECT_CALL(*mDataLoader, create(_, _, _, _)).Times(1); + EXPECT_CALL(*mDataLoader, start(_)).Times(1); + EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); + EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); + // System data loader to get rid of readlog timeout callback. + mDataLoaderParcel.packageName = "android"; + TemporaryDir tempDir; + int storageId = + mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel, + IncrementalService::CreateOptions::CreateNew); + ASSERT_GE(storageId, 0); + ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {}, + {}, {})); + + // Started. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); + + // IfsState callback present. + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); + auto callback = mTimedQueue->mWhat; + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + + // Not loaded yet. + EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) + .WillOnce(Return(incfs::LoadingState::MissingBlocks)); + + // Send the callback, should not do anything. + callback(); + + // Still started. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); + + // Still present. + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); + callback = mTimedQueue->mWhat; + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + + // Fully loaded. + EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)).WillOnce(Return(incfs::LoadingState::Full)); + // Expect the unbind. + EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(1); + + callback(); + + // Destroyed. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_DESTROYED); +} + +TEST_F(IncrementalServiceTest, testStartDataLoaderUnbindOnAllDoneWithReadlogs) { + mFs->hasFiles(); + + // Readlogs. + mVold->setIncFsMountOptionsSuccess(); + mAppOpsManager->checkPermissionSuccess(); + + const auto stateUpdateInterval = 1s; + + EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _, _)).Times(1); + // No unbinding just yet. + EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(0); + EXPECT_CALL(*mDataLoader, create(_, _, _, _)).Times(1); + EXPECT_CALL(*mDataLoader, start(_)).Times(1); + EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); + EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); + // System data loader to get rid of readlog timeout callback. + mDataLoaderParcel.packageName = "android"; + TemporaryDir tempDir; + int storageId = + mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel, + IncrementalService::CreateOptions::CreateNew); + ASSERT_GE(storageId, 0); + ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {}, + {}, {})); + + // Started. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); + + // IfsState callback present. + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); + auto callback = mTimedQueue->mWhat; + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + + // Not loaded yet. + EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) + .WillOnce(Return(incfs::LoadingState::MissingBlocks)); + + // Send the callback, should not do anything. + callback(); + + // Still started. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); + + // Still present. + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); + callback = mTimedQueue->mWhat; + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + + // Fully loaded. + EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) + .WillOnce(Return(incfs::LoadingState::Full)) + .WillOnce(Return(incfs::LoadingState::Full)); + // But with readlogs. + ASSERT_GE(mDataLoader->setStorageParams(true), 0); + + // Send the callback, still nothing. + callback(); + + // Still started. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_STARTED); + + // Still present. + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); + ASSERT_EQ(mTimedQueue->mAfter, stateUpdateInterval); + callback = mTimedQueue->mWhat; + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); + + // Disable readlogs and expect the unbind. + EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(1); + ASSERT_GE(mDataLoader->setStorageParams(false), 0); + + callback(); + + // Destroyed. + ASSERT_EQ(mDataLoader->status(), IDataLoaderStatusListener::DATA_LOADER_DESTROYED); +} + TEST_F(IncrementalServiceTest, testRegisterStorageHealthListenerSuccess) { mIncFs->openMountSuccess(); sp<NiceMock<MockStorageHealthListener>> listener{new NiceMock<MockStorageHealthListener>}; @@ -1801,7 +1944,7 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsTooShort) { EXPECT_CALL(*mDataLoader, start(_)).Times(1); EXPECT_CALL(*mDataLoader, destroy(_)).Times(1); EXPECT_CALL(*mIncFs, setUidReadTimeouts(_, _)).Times(0); - EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(1); + EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2); EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2); TemporaryDir tempDir; int storageId = @@ -1829,7 +1972,6 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { EXPECT_CALL(*mIncFs, isEverythingFullyLoaded(_)) .WillOnce(Return(incfs::LoadingState::MissingBlocks)) .WillOnce(Return(incfs::LoadingState::MissingBlocks)) - .WillOnce(Return(incfs::LoadingState::Full)) .WillOnce(Return(incfs::LoadingState::Full)); // Mark DataLoader as 'system' so that readlogs don't pollute the timed queue. @@ -1846,10 +1988,10 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { { // Timed callback present -> 0 progress. - ASSERT_EQ(storageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1)); const auto timedCallback = mTimedQueue->mWhat; - mTimedQueue->clearJob(storageId); + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); // Call it again. timedCallback(); @@ -1857,10 +1999,10 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { { // Still present -> some progress. - ASSERT_EQ(storageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1)); const auto timedCallback = mTimedQueue->mWhat; - mTimedQueue->clearJob(storageId); + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); // Fully loaded but readlogs collection enabled. ASSERT_GE(mDataLoader->setStorageParams(true), 0); @@ -1871,10 +2013,10 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) { { // Still present -> fully loaded + readlogs. - ASSERT_EQ(storageId, mTimedQueue->mId); + ASSERT_EQ(IncrementalService::kMaxStorageId, mTimedQueue->mId); ASSERT_GE(mTimedQueue->mAfter, std::chrono::seconds(1)); const auto timedCallback = mTimedQueue->mWhat; - mTimedQueue->clearJob(storageId); + mTimedQueue->clearJob(IncrementalService::kMaxStorageId); // Now disable readlogs. ASSERT_GE(mDataLoader->setStorageParams(false), 0); |