Additional lock to avoid access to deleted object.
Bug: b/155692497
Fixes: 155692497
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: Ie02012bd5a6c0640d54c5390d3726a5405042408
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 992a4ef..c3c2157 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -1670,9 +1670,15 @@
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::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 @@
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 @@
int currentStatus;
int targetStatus;
{
- std::unique_lock lock(mStatusMutex);
+ std::unique_lock lock(mMutex);
currentStatus = mCurrentStatus;
targetStatus = mTargetStatus;
}
@@ -1828,8 +1827,9 @@
}
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 @@
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 @@
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();
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index d5c612d..cf310b1 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -188,17 +188,17 @@
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;