Merge "Potential deadlock mitigation." into sc-dev

services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp

@@ -44,7 +44,6 @@ namespace android {
 namespace {
 
 using android::base::borrowed_fd;
-using android::base::ReadFully;
 using android::base::unique_fd;
 
 using namespace std::literals;
@@ -173,7 +172,7 @@ static inline int32_t readLEInt32(borrowed_fd fd) {
 static inline std::vector<char> readBytes(borrowed_fd fd) {
     int32_t size = readLEInt32(fd);
     std::vector<char> result(size);
-    ReadFully(fd, result.data(), size);
+    android::base::ReadFully(fd, result.data(), size);
     return result;
 }
 
@@ -569,7 +568,7 @@ private:
         // Awaiting adb handshake.
         char okay_buf[OKAY.size()];
         if (!android::base::ReadFully(inout, okay_buf, OKAY.size())) {
-            ALOGE("Failed to receive OKAY. Abort.");
+            ALOGE("Failed to receive OKAY. Abort. Error %d", errno);
             return false;
         }
         if (std::string_view(okay_buf, OKAY.size()) != OKAY) {
@@ -693,12 +692,12 @@ private:
                 continue;
             }
             if (res < 0) {
-                ALOGE("Failed to poll. Abort.");
+                ALOGE("Failed to poll. Abort. Error %d", res);
                 mStatusListener->reportStatus(DATA_LOADER_UNRECOVERABLE);
                 break;
             }
             if (res == mEventFd) {
-                ALOGE("Received stop signal. Sending EXIT to server.");
+                ALOGE("DataLoader requested to stop. Sending EXIT to server.");
                 sendRequest(inout, EXIT);
                 break;
             }
@@ -712,7 +711,7 @@ private:
                 auto header = readHeader(remainingData);
                 if (header.fileIdx == -1 && header.blockType == 0 && header.compressionType == 0 &&
                     header.blockIdx == 0 && header.blockSize == 0) {
-                    ALOGI("Stop signal received. Sending exit command (remaining bytes: %d).",
+                    ALOGI("Stop command received. Sending exit command (remaining bytes: %d).",
                           int(remainingData.size()));
 
                     sendRequest(inout, EXIT);
@@ -721,16 +720,15 @@ private:
                 }
                 if (header.fileIdx < 0 || header.blockSize <= 0 || header.blockType < 0 ||
                     header.compressionType < 0 || header.blockIdx < 0) {
-                    ALOGE("invalid header received. Abort.");
+                    ALOGE("Invalid header received. Abort.");
                     mStopReceiving = true;
                     break;
                 }
+
                 const FileIdx fileIdx = header.fileIdx;
                 const android::dataloader::FileId fileId = convertFileIndexToFileId(mode, fileIdx);
                 if (!android::incfs::isValidFileId(fileId)) {
-                    ALOGE("Unknown data destination for file ID %d. "
-                          "Ignore.",
-                          header.fileIdx);
+                    ALOGE("Unknown data destination for file ID %d. Ignore.", header.fileIdx);
                     continue;
                 }
 
@@ -738,7 +736,7 @@ private:
                 if (writeFd < 0) {
                     writeFd.reset(this->mIfs->openForSpecialOps(fileId).release());
                     if (writeFd < 0) {
-                        ALOGE("Failed to open file %d for writing (%d). Aborting.", header.fileIdx,
+                        ALOGE("Failed to open file %d for writing (%d). Abort.", header.fileIdx,
                               -writeFd);
                         break;
                     }

services/incremental/IncrementalService.cpp

@@ -66,6 +66,8 @@ struct Constants {
     static constexpr auto blockSize = 4096;
     static constexpr auto systemPackage = "android"sv;
 
+    static constexpr auto userStatusDelay = 100ms;
+
     static constexpr auto progressUpdateInterval = 1000ms;
     static constexpr auto perUidTimeoutOffset = progressUpdateInterval * 2;
     static constexpr auto minPerUidTimeout = progressUpdateInterval * 3;
@@ -2306,13 +2308,24 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
         LOG(ERROR) << "Mount ID mismatch: expected " << id() << ", but got: " << mountId;
         return binder::Status::fromServiceSpecificError(-EPERM, "Mount ID mismatch.");
     }
+    if (newStatus == IDataLoaderStatusListener::DATA_LOADER_UNRECOVERABLE) {
+        // User-provided status, let's postpone the handling to avoid possible deadlocks.
+        mService.addTimedJob(*mService.mTimedQueue, id(), Constants::userStatusDelay,
+                             [this, newStatus]() { setCurrentStatus(newStatus); });
+        return binder::Status::ok();
+    }
 
+    setCurrentStatus(newStatus);
+    return binder::Status::ok();
+}
+
+void IncrementalService::DataLoaderStub::setCurrentStatus(int newStatus) {
     int targetStatus, oldStatus;
     DataLoaderStatusListener listener;
     {
         std::unique_lock lock(mMutex);
         if (mCurrentStatus == newStatus) {
-            return binder::Status::ok();
+            return;
         }
 
         oldStatus = mCurrentStatus;
@@ -2332,14 +2345,12 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
                << newStatus << " (target " << targetStatus << ")";
 
     if (listener) {
-        listener->onStatusChanged(mountId, newStatus);
+        listener->onStatusChanged(id(), newStatus);
     }
 
     fsmStep();
 
     mStatusCondition.notify_all();
-
-    return binder::Status::ok();
 }
 
 binder::Status IncrementalService::DataLoaderStub::reportStreamHealth(MountId mountId,

services/incremental/IncrementalService.h

@@ -234,6 +234,8 @@ private:
         binder::Status onStatusChanged(MountId mount, int newStatus) final;
         binder::Status reportStreamHealth(MountId mount, int newStatus) final;
 
+        void setCurrentStatus(int newStatus);
+
         sp<content::pm::IDataLoader> getDataLoader();
 
         bool bind();