Unbind from DataLoader when not needed anymore.
+ simplify adding new callbacks on storage state
+ streamline lock story for ifs members
Bug: 183101753
Fixes: 183101753
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageManagerServiceTest ChecksumsTest
Change-Id: I86fffa7101eeb42ebccca67ae7f5d133c1ab9dfa
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index a88f2b4..932e997 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -377,6 +377,7 @@
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 @@
}
bool IncrementalService::needStartDataLoaderLocked(IncFsMount& ifs) {
+ if (!ifs.dataLoaderStub) {
+ return false;
+ }
if (ifs.dataLoaderStub->isSystemDataLoader()) {
return true;
}
@@ -439,6 +443,8 @@
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 @@
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 @@
setUidReadTimeouts(storageId, std::move(perUidReadTimeouts));
}
- // Re-initialize DataLoader.
- std::unique_lock l(mLock);
- const auto ifs = getIfsLocked(storageId);
- if (!ifs) {
- return false;
- }
- if (ifs->dataLoaderStub) {
- ifs->dataLoaderStub->cleanupResources();
- ifs->dataLoaderStub = {};
- }
- l.unlock();
+ IfsMountPtr ifs;
+ DataLoaderStubPtr dataLoaderStub;
- // DataLoader.
- auto dataLoaderStub =
- prepareDataLoader(*ifs, std::move(dataLoaderParams), std::move(statusListener),
- healthCheckParams, std::move(healthListener));
- CHECK(dataLoaderStub);
+ // Re-initialize DataLoader.
+ {
+ ifs = getIfs(storageId);
+ if (!ifs) {
+ return false;
+ }
+
+ std::unique_lock l(ifs->lock);
+ dataLoaderStub = std::exchange(ifs->dataLoaderStub, nullptr);
+ }
+
+ if (dataLoaderStub) {
+ dataLoaderStub->cleanupResources();
+ 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 @@
<< 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 @@
}
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 @@
return;
}
- setStorageParams(storageId, /*enableReadLogs=*/false);
+ disableReadLogsLocked(*ifs);
}
int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLogs) {
@@ -764,61 +787,66 @@
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;
- }
-
- // 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);
- }
-
- // 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);
- }
-
- // 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;
- }
+ std::unique_lock l(ifs->lock);
+ if (!enableReadLogs) {
+ return disableReadLogsLocked(*ifs);
}
- if (auto status = applyStorageParams(ifs, enableReadLogs); !status.isOk()) {
- LOG(ERROR) << "applyStorageParams failed: " << status.toString8();
+ if (!ifs->readLogsAllowed()) {
+ LOG(ERROR) << "enableReadLogs failed, readlogs disallowed for storageId: " << storageId;
+ return -EPERM;
+ }
+
+ 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) << "enableReadLogs failed, readlogs can't be enabled at this time, storageId: "
+ << storageId;
+ return -EPERM;
+ }
+
+ 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 @@
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 @@
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 @@
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;
+ }
+ if (state.error) {
+ // Something is wrong, abort.
+ return false;
}
// Still loading?
- const auto state = isMountFullyLoaded(storage);
- if (int(state) < 0) {
- // Something is wrong, abort.
- return clearUidReadTimeouts(storage);
- }
-
- if (state == incfs::LoadingState::Full) {
- // Fully loaded, check readLogs collection.
- const auto ifs = getIfs(storage);
- if (!ifs->readLogsEnabled()) {
- return clearUidReadTimeouts(storage);
- }
+ 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 @@
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 @@
}
}
-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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
resetHealthControl();
mService.removeTimedJobs(*mService.mTimedQueue, mId);
}
+ mService.removeIfsStateCallbacks(mId);
requestDestroy();
@@ -2758,7 +2894,7 @@
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 4a5db06..a8f32de 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -74,6 +74,17 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 54bc95d..de8822d 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -140,9 +140,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
{
// 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 @@
{
// 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 @@
{
// 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);