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

Additional lock to avoid access to deleted object.

Browse Source 
Bug: b/155692497
Fixes: 155692497
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: Ie02012bd5a6c0640d54c5390d3726a5405042408
This commit is contained in:
Alex Buynytskyy
2020-05-04 18:39:58 -07:00
parent ed2789354a
commit b0ea448eeb
2 changed files with 17 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
services/incremental/IncrementalService.cpp
View File

@@ -1670,9 +1670,15 @@ IncrementalService::DataLoaderStub::~DataLoaderStub() = default;
void IncrementalService::DataLoaderStub::cleanupResources() {
requestDestroy();
auto now = Clock::now();
std::unique_lock lock(mMutex);
mParams = {};
mControl = {};
waitForStatus(IDataLoaderStatusListener::DATA_LOADER_DESTROYED, std::chrono::seconds(60));
mStatusCondition.wait_until(lock, now + 60s, [this] {
return mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
});
mListener = {};
mId = kInvalidStorageId;
}
@@ -1706,7 +1712,7 @@ bool IncrementalService::DataLoaderStub::requestDestroy() {
bool IncrementalService::DataLoaderStub::setTargetStatus(int newStatus) {
int oldStatus, curStatus;
{
std::unique_lock lock(mStatusMutex);
std::unique_lock lock(mMutex);
oldStatus = mTargetStatus;
curStatus = mCurrentStatus;
setTargetStatusLocked(newStatus);
@@ -1721,13 +1727,6 @@ void IncrementalService::DataLoaderStub::setTargetStatusLocked(int status) {
mTargetStatusTs = Clock::now();
}
bool IncrementalService::DataLoaderStub::waitForStatus(int status, Clock::duration duration) {
auto now = Clock::now();
std::unique_lock lock(mStatusMutex);
return mStatusCondition.wait_until(lock, now + duration,
[this, status] { return mCurrentStatus == status; });
}
bool IncrementalService::DataLoaderStub::bind() {
bool result = false;
auto status = mService.mDataLoaderManager->bindToDataLoader(mId, mParams, this, &result);
@@ -1776,7 +1775,7 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
int currentStatus;
int targetStatus;
{
std::unique_lock lock(mStatusMutex);
std::unique_lock lock(mMutex);
currentStatus = mCurrentStatus;
targetStatus = mTargetStatus;
}
@@ -1828,8 +1827,9 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
}
int targetStatus, oldStatus;
DataLoaderStatusListener listener;
{
std::unique_lock lock(mStatusMutex);
std::unique_lock lock(mMutex);
if (mCurrentStatus == newStatus) {
return binder::Status::ok();
}
@@ -1838,6 +1838,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
mCurrentStatus = newStatus;
targetStatus = mTargetStatus;
listener = mListener;
if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE) {
// For unavailable, reset target status.
setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE);
@@ -1847,8 +1849,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
LOG(DEBUG) << "Current status update for DataLoader " << mId << ": " << oldStatus << " -> "
<< newStatus << " (target " << targetStatus << ")";
if (mListener) {
mListener->onStatusChanged(mountId, newStatus);
if (listener) {
listener->onStatusChanged(mountId, newStatus);
}
fsmStep();

4
services/incremental/IncrementalService.h
View File

@@ -188,17 +188,17 @@ private:
bool setTargetStatus(int status);
void setTargetStatusLocked(int status);
bool waitForStatus(int status, Clock::duration duration);
bool fsmStep();
IncrementalService& mService;
std::mutex mMutex;
MountId mId = kInvalidStorageId;
content::pm::DataLoaderParamsParcel mParams;
content::pm::FileSystemControlParcel mControl;
DataLoaderStatusListener mListener;
std::mutex mStatusMutex;
std::condition_variable mStatusCondition;
int mCurrentStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;
int mTargetStatus = content::pm::IDataLoaderStatusListener::DATA_LOADER_DESTROYED;