Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Refactor: move dataLoader details to a separate class." into rvc-dev

Browse Source
This commit is contained in:
TreeHugger Robot
2020-04-10 16:53:56 +00:00
committed by Android (Google) Code Review
parent 0cd8012b73 0ea4ff4d97
commit 771027008b
3 changed files with 184 additions and 129 deletions
Download Patch File Download Diff File Expand all files Collapse all files

198
services/incremental/IncrementalService.cpp
View File

@@ -164,7 +164,9 @@ const bool IncrementalService::sEnablePerfLogging =
android::base::GetBoolProperty("incremental.perflogging", false);
IncrementalService::IncFsMount::~IncFsMount() {
incrementalService.mDataLoaderManager->destroyDataLoader(mountId);
if (dataLoaderStub) {
dataLoaderStub->destroy();
}
LOG(INFO) << "Unmounting and cleaning up mount " << mountId << " with root '" << root << '\'';
for (auto&& [target, _] : bindPoints) {
LOG(INFO) << "\tbind: " << target;
@@ -289,9 +291,12 @@ void IncrementalService::onDump(int fd) {
dprintf(fd, "\t\tmountId: %d\n", mnt.mountId);
dprintf(fd, "\t\troot: %s\n", mnt.root.c_str());
dprintf(fd, "\t\tnextStorageDirNo: %d\n", mnt.nextStorageDirNo.load());
dprintf(fd, "\t\tdataLoaderStatus: %d\n", mnt.dataLoaderStatus.load());
{
const auto& params = mnt.dataLoaderParams;
if (mnt.dataLoaderStub) {
const auto& dataLoaderStub = *mnt.dataLoaderStub;
dprintf(fd, "\t\tdataLoaderStatus: %d\n", dataLoaderStub.status());
dprintf(fd, "\t\tdataLoaderStartRequested: %s\n",
dataLoaderStub.startRequested() ? "true" : "false");
const auto& params = dataLoaderStub.params();
dprintf(fd, "\t\tdataLoaderParams:\n");
dprintf(fd, "\t\t\ttype: %s\n", toString(params.type).c_str());
dprintf(fd, "\t\t\tpackageName: %s\n", params.packageName.c_str());
@@ -322,10 +327,9 @@ void IncrementalService::onDump(int fd) {
}
}
std::optional<std::future<void>> IncrementalService::onSystemReady() {
std::promise<void> threadFinished;
void IncrementalService::onSystemReady() {
if (mSystemReady.exchange(true)) {
return {};
return;
}
std::vector<IfsMountPtr> mounts;
@@ -339,8 +343,8 @@ std::optional<std::future<void>> IncrementalService::onSystemReady() {
}
}
/* TODO(b/151241369): restore data loaders on reboot.
std::thread([this, mounts = std::move(mounts)]() {
/* TODO(b/151241369): restore data loaders on reboot.
for (auto&& ifs : mounts) {
if (prepareDataLoader(*ifs)) {
LOG(INFO) << "Successfully started data loader for mount " << ifs->mountId;
@@ -349,10 +353,8 @@ std::optional<std::future<void>> IncrementalService::onSystemReady() {
LOG(WARNING) << "Failed to start data loader for mount " << ifs->mountId;
}
}
*/
mPrepareDataLoaders.set_value_at_thread_exit();
}).detach();
return mPrepareDataLoaders.get_future();
*/
}
auto IncrementalService::getStorageSlotLocked() -> MountMap::iterator {
@@ -469,15 +471,13 @@ StorageId IncrementalService::createStorage(
return kInvalidStorageId;
}
ifs->dataLoaderParams = std::move(dataLoaderParams);
{
metadata::Mount m;
m.mutable_storage()->set_id(ifs->mountId);
m.mutable_loader()->set_type((int)ifs->dataLoaderParams.type);
m.mutable_loader()->set_package_name(ifs->dataLoaderParams.packageName);
m.mutable_loader()->set_class_name(ifs->dataLoaderParams.className);
m.mutable_loader()->set_arguments(ifs->dataLoaderParams.arguments);
m.mutable_loader()->set_type((int)dataLoaderParams.type);
m.mutable_loader()->set_package_name(dataLoaderParams.packageName);
m.mutable_loader()->set_class_name(dataLoaderParams.className);
m.mutable_loader()->set_arguments(dataLoaderParams.arguments);
const auto metadata = m.SerializeAsString();
m.mutable_loader()->release_arguments();
m.mutable_loader()->release_class_name();
@@ -505,14 +505,20 @@ StorageId IncrementalService::createStorage(
// Done here as well, all data structures are in good state.
secondCleanupOnFailure.release();
if (!prepareDataLoader(*ifs, &dataLoaderStatusListener)) {
LOG(ERROR) << "prepareDataLoader() failed";
deleteStorageLocked(*ifs, std::move(l));
return kInvalidStorageId;
}
auto dataLoaderStub =
prepareDataLoader(*ifs, std::move(dataLoaderParams), &dataLoaderStatusListener);
CHECK(dataLoaderStub);
mountIt->second = std::move(ifs);
l.unlock();
if (mSystemReady.load(std::memory_order_relaxed) && !dataLoaderStub->create()) {
// failed to create data loader
LOG(ERROR) << "initializeDataLoader() failed";
deleteStorage(dataLoaderStub->id());
return kInvalidStorageId;
}
LOG(INFO) << "created storage " << mountId;
return mountId;
}
@@ -586,10 +592,10 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
return -EINVAL;
}
const auto& params = ifs->dataLoaderStub->params();
if (enableReadLogs) {
if (auto status =
mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage,
ifs->dataLoaderParams.packageName.c_str());
if (auto status = mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage,
params.packageName.c_str());
!status.isOk()) {
LOG(ERROR) << "checkPermission failed: " << status.toString8();
return fromBinderStatus(status);
@@ -602,7 +608,7 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
}
if (enableReadLogs) {
registerAppOpsCallback(ifs->dataLoaderParams.packageName);
registerAppOpsCallback(params.packageName);
}
return 0;
@@ -985,34 +991,19 @@ std::vector<std::string> IncrementalService::listFiles(StorageId storage) const
}
bool IncrementalService::startLoading(StorageId storage) const {
DataLoaderStubPtr dataLoaderStub;
{
std::unique_lock l(mLock);
const auto& ifs = getIfsLocked(storage);
if (!ifs) {
return false;
}
if (ifs->dataLoaderStatus != IDataLoaderStatusListener::DATA_LOADER_CREATED) {
ifs->dataLoaderStartRequested = true;
return true;
dataLoaderStub = ifs->dataLoaderStub;
if (!dataLoaderStub) {
return false;
}
}
return startDataLoader(storage);
}
bool IncrementalService::startDataLoader(MountId mountId) const {
sp<IDataLoader> dataloader;
auto status = mDataLoaderManager->getDataLoader(mountId, &dataloader);
if (!status.isOk()) {
return false;
}
if (!dataloader) {
return false;
}
status = dataloader->start(mountId);
if (!status.isOk()) {
return false;
}
return true;
return dataLoaderStub->start();
}
void IncrementalService::mountExistingImages() {
@@ -1058,13 +1049,13 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
mNextId = std::max(mNextId, ifs->mountId + 1);
// DataLoader params
DataLoaderParamsParcel dataLoaderParams;
{
auto& dlp = ifs->dataLoaderParams;
const auto& loader = mount.loader();
dlp.type = (android::content::pm::DataLoaderType)loader.type();
dlp.packageName = loader.package_name();
dlp.className = loader.class_name();
dlp.arguments = loader.arguments();
dataLoaderParams.type = (android::content::pm::DataLoaderType)loader.type();
dataLoaderParams.packageName = loader.package_name();
dataLoaderParams.className = loader.class_name();
dataLoaderParams.arguments = loader.arguments();
}
std::vector<std::pair<std::string, metadata::BindPoint>> bindPoints;
@@ -1136,17 +1127,13 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
return true;
}
bool IncrementalService::prepareDataLoader(IncrementalService::IncFsMount& ifs,
const DataLoaderStatusListener* externalListener) {
if (!mSystemReady.load(std::memory_order_relaxed)) {
std::unique_lock l(ifs.lock);
return true; // eventually...
}
IncrementalService::DataLoaderStubPtr IncrementalService::prepareDataLoader(
IncrementalService::IncFsMount& ifs, DataLoaderParamsParcel&& params,
const DataLoaderStatusListener* externalListener) {
std::unique_lock l(ifs.lock);
if (ifs.dataLoaderStatus != -1) {
if (ifs.dataLoaderStub) {
LOG(INFO) << "Skipped data loader preparation because it already exists";
return true;
return ifs.dataLoaderStub;
}
FileSystemControlParcel fsControlParcel;
@@ -1156,17 +1143,10 @@ bool IncrementalService::prepareDataLoader(IncrementalService::IncFsMount& ifs,
base::unique_fd(::dup(ifs.control.pendingReads())));
fsControlParcel.incremental->log.reset(base::unique_fd(::dup(ifs.control.logs())));
fsControlParcel.service = new IncrementalServiceConnector(*this, ifs.mountId);
sp<IncrementalDataLoaderListener> listener =
new IncrementalDataLoaderListener(*this,
externalListener ? *externalListener
: DataLoaderStatusListener());
bool created = false;
auto status = mDataLoaderManager->initializeDataLoader(ifs.mountId, ifs.dataLoaderParams, fsControlParcel, listener, &created);
if (!status.isOk() || !created) {
LOG(ERROR) << "Failed to create a data loader for mount " << ifs.mountId;
return false;
}
return true;
ifs.dataLoaderStub = new DataLoaderStub(*this, ifs.mountId, std::move(params),
std::move(fsControlParcel), externalListener);
return ifs.dataLoaderStub;
}
template <class Duration>
@@ -1377,7 +1357,7 @@ 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->dataLoaderParams.packageName == packageName) {
if (ifs->mountId == id && ifs->dataLoaderStub->params().packageName == packageName) {
affected.push_back(ifs);
}
}
@@ -1387,37 +1367,79 @@ void IncrementalService::onAppOpChanged(const std::string& packageName) {
}
}
binder::Status IncrementalService::IncrementalDataLoaderListener::onStatusChanged(MountId mountId,
int newStatus) {
if (externalListener) {
// Give an external listener a chance to act before we destroy something.
externalListener->onStatusChanged(mountId, newStatus);
IncrementalService::DataLoaderStub::~DataLoaderStub() {
CHECK(mStatus == -1 || mStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED)
<< "Dataloader has to be destroyed prior to destructor: " << mId
<< ", status: " << mStatus;
}
bool IncrementalService::DataLoaderStub::create() {
bool created = false;
auto status = mService.mDataLoaderManager->initializeDataLoader(mId, mParams, mControl, this,
&created);
if (!status.isOk() || !created) {
LOG(ERROR) << "Failed to create a data loader for mount " << mId;
return false;
}
return true;
}
bool IncrementalService::DataLoaderStub::start() {
if (mStatus != IDataLoaderStatusListener::DATA_LOADER_CREATED) {
mStartRequested = true;
return true;
}
sp<IDataLoader> dataloader;
auto status = mService.mDataLoaderManager->getDataLoader(mId, &dataloader);
if (!status.isOk()) {
return false;
}
if (!dataloader) {
return false;
}
status = dataloader->start(mId);
if (!status.isOk()) {
return false;
}
return true;
}
void IncrementalService::DataLoaderStub::destroy() {
mDestroyRequested = true;
mService.mDataLoaderManager->destroyDataLoader(mId);
}
binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mountId, int newStatus) {
if (mStatus == newStatus) {
return binder::Status::ok();
}
if (mListener) {
// Give an external listener a chance to act before we destroy something.
mListener->onStatusChanged(mountId, newStatus);
}
bool startRequested = false;
{
std::unique_lock l(incrementalService.mLock);
const auto& ifs = incrementalService.getIfsLocked(mountId);
std::unique_lock l(mService.mLock);
const auto& ifs = mService.getIfsLocked(mountId);
if (!ifs) {
LOG(WARNING) << "Received data loader status " << int(newStatus)
<< " for unknown mount " << mountId;
return binder::Status::ok();
}
ifs->dataLoaderStatus = newStatus;
mStatus = newStatus;
if (newStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED) {
ifs->dataLoaderStatus = IDataLoaderStatusListener::DATA_LOADER_STOPPED;
incrementalService.deleteStorageLocked(*ifs, std::move(l));
if (!mDestroyRequested && newStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED) {
mService.deleteStorageLocked(*ifs, std::move(l));
return binder::Status::ok();
}
startRequested = ifs->dataLoaderStartRequested;
}
switch (newStatus) {
case IDataLoaderStatusListener::DATA_LOADER_CREATED: {
if (startRequested) {
incrementalService.startDataLoader(mountId);
if (mStartRequested) {
start();
}
break;
}

66
services/incremental/IncrementalService.h
View File

@@ -60,7 +60,8 @@ using Clock = std::chrono::steady_clock;
using TimePoint = std::chrono::time_point<Clock>;
using Seconds = std::chrono::seconds;
using DataLoaderStatusListener = ::android::sp<::android::content::pm::IDataLoaderStatusListener>;
using IDataLoaderStatusListener = ::android::content::pm::IDataLoaderStatusListener;
using DataLoaderStatusListener = ::android::sp<IDataLoaderStatusListener>;
class IncrementalService final {
public:
@@ -95,7 +96,7 @@ public:
void onDump(int fd);
std::optional<std::future<void>> onSystemReady();
void onSystemReady();
StorageId createStorage(std::string_view mountPoint, DataLoaderParamsParcel&& dataLoaderParams,
const DataLoaderStatusListener& dataLoaderStatusListener,
@@ -134,19 +135,6 @@ public:
bool configureNativeBinaries(StorageId storage, std::string_view apkFullPath,
std::string_view libDirRelativePath, std::string_view abi);
class IncrementalDataLoaderListener : public android::content::pm::BnDataLoaderStatusListener {
public:
IncrementalDataLoaderListener(IncrementalService& incrementalService,
DataLoaderStatusListener externalListener)
: incrementalService(incrementalService), externalListener(externalListener) {}
// Callbacks interface
binder::Status onStatusChanged(MountId mount, int newStatus) final;
private:
IncrementalService& incrementalService;
DataLoaderStatusListener externalListener;
};
class AppOpsListener : public android::BnAppOpsCallback {
public:
AppOpsListener(IncrementalService& incrementalService, std::string packageName) : incrementalService(incrementalService), packageName(std::move(packageName)) {}
@@ -171,6 +159,45 @@ public:
private:
static const bool sEnablePerfLogging;
struct IncFsMount;
class DataLoaderStub : public android::content::pm::BnDataLoaderStatusListener {
public:
DataLoaderStub(IncrementalService& service, MountId id, DataLoaderParamsParcel&& params,
FileSystemControlParcel&& control,
const DataLoaderStatusListener* externalListener)
: mService(service),
mId(id),
mParams(std::move(params)),
mControl(std::move(control)),
mListener(externalListener ? *externalListener : DataLoaderStatusListener()) {}
~DataLoaderStub();
bool create();
bool start();
void destroy();
// accessors
MountId id() const { return mId; }
const DataLoaderParamsParcel& params() const { return mParams; }
int status() const { return mStatus.load(); }
bool startRequested() const { return mStartRequested; }
private:
binder::Status onStatusChanged(MountId mount, int newStatus) final;
IncrementalService& mService;
MountId const mId;
DataLoaderParamsParcel const mParams;
FileSystemControlParcel const mControl;
DataLoaderStatusListener const mListener;
std::atomic<int> mStatus = -1;
bool mStartRequested = false;
bool mDestroyRequested = false;
};
using DataLoaderStubPtr = sp<DataLoaderStub>;
struct IncFsMount {
struct Bind {
StorageId storage;
@@ -194,10 +221,8 @@ private:
/*const*/ MountId mountId;
StorageMap storages;
BindMap bindPoints;
DataLoaderParamsParcel dataLoaderParams;
DataLoaderStubPtr dataLoaderStub;
std::atomic<int> nextStorageDirNo{0};
std::atomic<int> dataLoaderStatus = -1;
bool dataLoaderStartRequested = false;
const IncrementalService& incrementalService;
IncFsMount(std::string root, MountId mountId, Control control,
@@ -232,8 +257,8 @@ private:
std::string&& source, std::string&& target, BindKind kind,
std::unique_lock<std::mutex>& mainLock);
bool prepareDataLoader(IncFsMount& ifs, const DataLoaderStatusListener* externalListener = nullptr);
bool startDataLoader(MountId mountId) const;
DataLoaderStubPtr prepareDataLoader(IncFsMount& ifs, DataLoaderParamsParcel&& params,
const DataLoaderStatusListener* externalListener = nullptr);
BindPathMap::const_iterator findStorageLocked(std::string_view path) const;
StorageId findStorageId(std::string_view path) const;
@@ -269,7 +294,6 @@ private:
std::atomic_bool mSystemReady = false;
StorageId mNextId = 0;
std::promise<void> mPrepareDataLoaders;
};
} // namespace android::incremental

49
services/incremental/test/IncrementalServiceTest.cpp
View File

@@ -131,6 +131,23 @@ public:
binder::Status(int32_t mountId, sp<IDataLoader>* _aidl_return));
MOCK_CONST_METHOD1(destroyDataLoader, binder::Status(int32_t mountId));
void initializeDataLoaderSuccess() {
ON_CALL(*this, initializeDataLoader(_, _, _, _, _))
.WillByDefault(Invoke(this, &MockDataLoaderManager::initializeDataLoaderOk));
}
void initializeDataLoaderFails() {
ON_CALL(*this, initializeDataLoader(_, _, _, _, _))
.WillByDefault(Return(
(binder::Status::fromExceptionCode(1, String8("failed to prepare")))));
}
void getDataLoaderSuccess() {
ON_CALL(*this, getDataLoader(_, _))
.WillByDefault(Invoke(this, &MockDataLoaderManager::getDataLoaderOk));
}
void destroyDataLoaderOk() {
ON_CALL(*this, destroyDataLoader(_))
.WillByDefault(Invoke(this, &MockDataLoaderManager::setDataLoaderStatusDestroyed));
}
binder::Status initializeDataLoaderOk(int32_t mountId, const DataLoaderParamsParcel& params,
const FileSystemControlParcel& control,
const sp<IDataLoaderStatusListener>& listener,
@@ -141,32 +158,22 @@ public:
*_aidl_return = true;
return binder::Status::ok();
}
binder::Status getDataLoaderOk(int32_t mountId, sp<IDataLoader>* _aidl_return) {
*_aidl_return = mDataLoader;
return binder::Status::ok();
}
void initializeDataLoaderFails() {
ON_CALL(*this, initializeDataLoader(_, _, _, _, _))
.WillByDefault(Return(
(binder::Status::fromExceptionCode(1, String8("failed to prepare")))));
}
void initializeDataLoaderSuccess() {
ON_CALL(*this, initializeDataLoader(_, _, _, _, _))
.WillByDefault(Invoke(this, &MockDataLoaderManager::initializeDataLoaderOk));
}
void getDataLoaderSuccess() {
ON_CALL(*this, getDataLoader(_, _))
.WillByDefault(Invoke(this, &MockDataLoaderManager::getDataLoaderOk));
}
void setDataLoaderStatusNotReady() {
mListener->onStatusChanged(mId, IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
}
void setDataLoaderStatusReady() {
mListener->onStatusChanged(mId, IDataLoaderStatusListener::DATA_LOADER_CREATED);
}
binder::Status setDataLoaderStatusDestroyed(int32_t id) {
if (mListener) {
mListener->onStatusChanged(id, IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
}
return binder::Status::ok();
}
int32_t setStorageParams(bool enableReadLogs) {
int32_t result = -1;
EXPECT_NE(mServiceConnector.get(), nullptr);
@@ -299,6 +306,7 @@ public:
mRootDir.path);
mDataLoaderParcel.packageName = "com.test";
mDataLoaderParcel.arguments = "uri";
mDataLoaderManager->destroyDataLoaderOk();
mIncrementalService->onSystemReady();
}
@@ -346,6 +354,7 @@ TEST_F(IncrementalServiceTest, testCreateStorageMountIncFsFails) {
TEST_F(IncrementalServiceTest, testCreateStorageMountIncFsInvalidControlParcel) {
mVold->mountIncFsInvalidControlParcel();
EXPECT_CALL(*mDataLoaderManager, initializeDataLoader(_, _, _, _, _)).Times(0);
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_)).Times(0);
TemporaryDir tempDir;
int storageId =
mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {},
@@ -357,7 +366,7 @@ TEST_F(IncrementalServiceTest, testCreateStorageMakeFileFails) {
mVold->mountIncFsSuccess();
mIncFs->makeFileFails();
EXPECT_CALL(*mDataLoaderManager, initializeDataLoader(_, _, _, _, _)).Times(0);
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_)).Times(0);
EXPECT_CALL(*mVold, unmountIncFs(_));
TemporaryDir tempDir;
int storageId =
@@ -371,7 +380,7 @@ TEST_F(IncrementalServiceTest, testCreateStorageBindMountFails) {
mIncFs->makeFileSuccess();
mVold->bindMountFails();
EXPECT_CALL(*mDataLoaderManager, initializeDataLoader(_, _, _, _, _)).Times(0);
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_)).Times(0);
EXPECT_CALL(*mVold, unmountIncFs(_));
TemporaryDir tempDir;
int storageId =
@@ -385,7 +394,7 @@ TEST_F(IncrementalServiceTest, testCreateStoragePrepareDataLoaderFails) {
mIncFs->makeFileSuccess();
mVold->bindMountSuccess();
mDataLoaderManager->initializeDataLoaderFails();
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_)).Times(1);
EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
TemporaryDir tempDir;
int storageId =
@@ -399,7 +408,7 @@ TEST_F(IncrementalServiceTest, testDeleteStorageSuccess) {
mIncFs->makeFileSuccess();
mVold->bindMountSuccess();
mDataLoaderManager->initializeDataLoaderSuccess();
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_)).Times(1);
EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
TemporaryDir tempDir;
int storageId =