Limit read log reporting to 2hrs for non-system DLs (non-adb).
Bug: 182477087
Test: atest IncrementalServiceTest
Change-Id: I98c9ed3a2e8a91d26bcb879ab7073903ff7bb2c5
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index c38d0b3..e3fbedd 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -86,6 +86,9 @@
static constexpr auto maxBindDelay = 10000s;
static constexpr auto bindDelayMultiplier = 10;
static constexpr auto bindDelayJitterDivider = 10;
+
+ // Max interval after system invoked the DL when readlog collection can be enabled.
+ static constexpr auto readLogsMaxInterval = 2h;
};
static const Constants& constants() {
@@ -290,6 +293,14 @@
::rmdir(path::c_str(root));
}
+void IncrementalService::IncFsMount::setReadLogsEnabled(bool value) {
+ if (value) {
+ flags |= StorageFlags::ReadLogsEnabled;
+ } else {
+ flags &= ~StorageFlags::ReadLogsEnabled;
+ }
+}
+
IncrementalService::IncrementalService(ServiceManagerWrapper&& sm, std::string_view rootDir)
: mVold(sm.getVoldService()),
mDataLoaderManager(sm.getDataLoaderManager()),
@@ -406,7 +417,7 @@
}
bool IncrementalService::needStartDataLoaderLocked(IncFsMount& ifs) {
- if (ifs.dataLoaderStub->params().packageName == Constants::systemPackage) {
+ if (ifs.dataLoaderStub->isSystemDataLoader()) {
return true;
}
@@ -658,7 +669,7 @@
return storageId;
}
-bool IncrementalService::startLoading(StorageId storage,
+bool IncrementalService::startLoading(StorageId storageId,
content::pm::DataLoaderParamsParcel&& dataLoaderParams,
const DataLoaderStatusListener& statusListener,
StorageHealthCheckParams&& healthCheckParams,
@@ -666,12 +677,12 @@
const std::vector<PerUidReadTimeouts>& perUidReadTimeouts) {
// Per Uid timeouts.
if (!perUidReadTimeouts.empty()) {
- setUidReadTimeouts(storage, perUidReadTimeouts);
+ setUidReadTimeouts(storageId, perUidReadTimeouts);
}
// Re-initialize DataLoader.
std::unique_lock l(mLock);
- const auto ifs = getIfsLocked(storage);
+ const auto ifs = getIfsLocked(storageId);
if (!ifs) {
return false;
}
@@ -686,6 +697,32 @@
std::move(healthCheckParams), &healthListener);
CHECK(dataLoaderStub);
+ if (dataLoaderStub->isSystemDataLoader()) {
+ // Readlogs from system dataloader (adb) can always be collected.
+ ifs->startLoadingTs = TimePoint::max();
+ } else {
+ // Assign time when installation wants the DL to start streaming.
+ const auto startLoadingTs = mClock->now();
+ ifs->startLoadingTs = startLoadingTs;
+ // Setup a callback to disable the readlogs after max interval.
+ addTimedJob(*mTimedQueue, storageId, Constants::readLogsMaxInterval,
+ [this, storageId, startLoadingTs]() {
+ const auto ifs = getIfs(storageId);
+ if (!ifs) {
+ LOG(WARNING) << "Can't disable the readlogs, invalid storageId: "
+ << storageId;
+ return;
+ }
+ if (ifs->startLoadingTs != startLoadingTs) {
+ LOG(INFO) << "Can't disable the readlogs, timestamp mismatch (new "
+ "installation?): "
+ << storageId;
+ return;
+ }
+ setStorageParams(*ifs, storageId, /*enableReadLogs=*/false);
+ });
+ }
+
return dataLoaderStub->requestStart();
}
@@ -735,11 +772,16 @@
LOG(ERROR) << "setStorageParams failed, invalid storageId: " << storageId;
return -EINVAL;
}
+ return setStorageParams(*ifs, storageId, enableReadLogs);
+}
- const auto& params = ifs->dataLoaderStub->params();
+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 disabled for storageId: " << storageId;
+ if (!ifs.readLogsAllowed()) {
+ LOG(ERROR) << "setStorageParams failed, readlogs disallowed for storageId: "
+ << storageId;
return -EPERM;
}
@@ -760,9 +802,19 @@
<< " 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;
+ }
}
- if (auto status = applyStorageParams(*ifs, enableReadLogs); !status.isOk()) {
+ if (auto status = applyStorageParams(ifs, enableReadLogs); !status.isOk()) {
LOG(ERROR) << "applyStorageParams failed: " << status.toString8();
return fromBinderStatus(status);
}
@@ -2222,6 +2274,10 @@
return dataloader;
}
+bool IncrementalService::DataLoaderStub::isSystemDataLoader() const {
+ return (params().packageName == Constants::systemPackage);
+}
+
bool IncrementalService::DataLoaderStub::requestCreate() {
return setTargetStatus(IDataLoaderStatusListener::DATA_LOADER_CREATED);
}
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 4eb5138..bc441c7 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -231,6 +231,7 @@
MountId id() const { return mId.load(std::memory_order_relaxed); }
const content::pm::DataLoaderParamsParcel& params() const { return mParams; }
+ bool isSystemDataLoader() const;
void setHealthListener(StorageHealthCheckParams&& healthCheckParams,
const StorageHealthListener* healthListener);
long elapsedMsSinceOldestPendingRead();
@@ -330,6 +331,7 @@
StorageMap storages;
BindMap bindPoints;
DataLoaderStubPtr dataLoaderStub;
+ TimePoint startLoadingTs = {};
std::atomic<int> nextStorageDirNo{0};
const IncrementalService& incrementalService;
@@ -348,12 +350,7 @@
void disallowReadLogs() { flags &= ~StorageFlags::ReadLogsAllowed; }
int32_t readLogsAllowed() const { return (flags & StorageFlags::ReadLogsAllowed); }
- void setReadLogsEnabled(bool value) {
- if (value)
- flags |= StorageFlags::ReadLogsEnabled;
- else
- flags &= ~StorageFlags::ReadLogsEnabled;
- }
+ void setReadLogsEnabled(bool value);
int32_t readLogsEnabled() const { return (flags & StorageFlags::ReadLogsEnabled); }
static void cleanupFilesystem(std::string_view root);
@@ -411,6 +408,8 @@
IncFsMount::StorageMap::const_iterator storageIt,
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 isFileFullyLoadedFromPath(const IncFsMount& ifs, std::string_view filePath) const;
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 25b34b56..bf798273 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -908,7 +908,7 @@
EXPECT_CALL(*mDataLoader, start(_)).Times(6);
EXPECT_CALL(*mDataLoader, destroy(_)).Times(1);
EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
- EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2);
+ EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(3);
TemporaryDir tempDir;
int storageId =
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
@@ -1119,7 +1119,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(4);
+ EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(5);
sp<NiceMock<MockStorageHealthListener>> listener{new NiceMock<MockStorageHealthListener>};
NiceMock<MockStorageHealthListener>* listenerMock = listener.get();
@@ -1292,6 +1292,147 @@
ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
}
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndTimedOut) {
+ mVold->setIncFsMountOptionsSuccess();
+ mAppOpsManager->checkPermissionSuccess();
+
+ const auto readLogsMaxInterval = 2h;
+
+ EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_));
+ EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+ // Enabling and then disabling readlogs.
+ EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(2);
+ EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(1);
+ // After setIncFsMountOptions succeeded expecting to start watching.
+ EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+ // Not expecting callback removal.
+ EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
+ EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(1);
+ TemporaryDir tempDir;
+ int storageId =
+ mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
+ IncrementalService::CreateOptions::CreateNew);
+ ASSERT_GE(storageId, 0);
+ ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
+ {}, {}));
+
+ // Disable readlogs callback present.
+ ASSERT_EQ(storageId, mTimedQueue->mId);
+ ASSERT_EQ(mTimedQueue->mAfter, readLogsMaxInterval);
+ auto callback = mTimedQueue->mWhat;
+ mTimedQueue->clearJob(storageId);
+
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+ // Now advance clock for 1hr.
+ mClock->advance(1h);
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+ // Now call the timed callback, it should turn off the readlogs.
+ callback();
+ // Now advance clock for 2hrs.
+ mClock->advance(readLogsMaxInterval);
+ ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
+}
+
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNoTimedOutForSystem) {
+ mVold->setIncFsMountOptionsSuccess();
+ mAppOpsManager->checkPermissionSuccess();
+
+ const auto readLogsMaxInterval = 2h;
+
+ EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_));
+ EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+ // Enabling and then disabling readlogs.
+ EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(3);
+ EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(0);
+ // After setIncFsMountOptions succeeded expecting to start watching.
+ EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+ // Not expecting callback removal.
+ EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
+ EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(0);
+ // System data loader.
+ 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), {}, {},
+ {}, {}));
+
+ // No readlogs callback.
+ ASSERT_EQ(mTimedQueue->mAfter, 0ms);
+ ASSERT_EQ(mTimedQueue->mWhat, nullptr);
+
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+ // Now advance clock for 1hr.
+ mClock->advance(1h);
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+ // Now advance clock for 2hrs.
+ mClock->advance(readLogsMaxInterval);
+ ASSERT_EQ(mDataLoader->setStorageParams(true), 0);
+}
+
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNewInstall) {
+ mVold->setIncFsMountOptionsSuccess();
+ mAppOpsManager->checkPermissionSuccess();
+
+ const auto readLogsMaxInterval = 2h;
+
+ EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(2);
+ EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+ // Enabling and then disabling readlogs.
+ EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(3);
+ EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(1);
+ // After setIncFsMountOptions succeeded expecting to start watching.
+ EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+ // Not expecting callback removal.
+ EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
+ EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2);
+ TemporaryDir tempDir;
+ int storageId =
+ mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
+ IncrementalService::CreateOptions::CreateNew);
+ ASSERT_GE(storageId, 0);
+
+ auto dataLoaderParcel = mDataLoaderParcel;
+ ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(dataLoaderParcel), {}, {},
+ {}, {}));
+
+ // Disable readlogs callback present.
+ ASSERT_EQ(storageId, mTimedQueue->mId);
+ ASSERT_EQ(mTimedQueue->mAfter, readLogsMaxInterval);
+ auto callback = mTimedQueue->mWhat;
+ mTimedQueue->clearJob(storageId);
+
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+ // Now advance clock for 1.5hrs.
+ mClock->advance(90min);
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+
+ // New installation.
+ ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
+ {}, {}));
+
+ // New callback present.
+ ASSERT_EQ(storageId, mTimedQueue->mId);
+ ASSERT_EQ(mTimedQueue->mAfter, readLogsMaxInterval);
+ auto callback2 = mTimedQueue->mWhat;
+ mTimedQueue->clearJob(storageId);
+
+ // Old callback should not disable readlogs (setIncFsMountOptions should be called only once).
+ callback();
+ // Advance clock for another 1.5hrs.
+ mClock->advance(90min);
+ // Still success even it's 3hrs past first install.
+ ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+
+ // New one should disable.
+ callback2();
+ // And timeout.
+ mClock->advance(90min);
+ ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
+}
+
TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndPermissionChanged) {
mVold->setIncFsMountOptionsSuccess();
mAppOpsManager->checkPermissionSuccess();
@@ -1675,7 +1816,7 @@
EXPECT_CALL(*mDataLoader, start(_)).Times(1);
EXPECT_CALL(*mDataLoader, destroy(_)).Times(1);
EXPECT_CALL(*mIncFs, setUidReadTimeouts(_, _)).Times(0);
- EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(0);
+ EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(1);
EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
TemporaryDir tempDir;
int storageId =
@@ -1702,6 +1843,9 @@
// Empty storage.
mIncFs->countFilledBlocksEmpty();
+ // Mark DataLoader as 'system' so that readlogs don't pollute the timed queue.
+ mDataLoaderParcel.packageName = "android";
+
TemporaryDir tempDir;
int storageId =
mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,