Additional lock to avoid access to deleted object.

Bug: b/155692497
Fixes: 155692497
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: Ie02012bd5a6c0640d54c5390d3726a5405042408

2 files changed, 17 insertions, 15 deletions

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 992a4ef76f04..c3c215730e4c 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -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();
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index d5c612daee58..cf310b15b6d9 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -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;