diff options
author | Alex Buynytskyy <alexbuy@google.com> | 2021-02-18 20:55:17 -0800 |
---|---|---|
committer | Alex Buynytskyy <alexbuy@google.com> | 2021-02-20 06:21:19 +0000 |
commit | 060c9d6869a698d9605942ee1e2074aeeabc5f79 (patch) | |
tree | d9bb517307501c48021dc3b8413b7a89826cf01e /services/incremental/IncrementalService.cpp | |
parent | 849587032600a159416035e4978b451969f1d695 (diff) |
Potential deadlock mitigation.
+better error logging
DataLoaders might report user statuses from lifecycle callbacks.
Immediate processing of such might introduce infinite loops/deadlocks e.g.
DataLoader_OnStop -> reportStatus(UNRECOVERABLE) -> fsmStep -> DataLoader_OnStop
Bug: 160634487
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageManagerServiceTest ChecksumsTest
Change-Id: Ic68657d7a8cd6c6855b6f5295276a42b3cb09117
Diffstat (limited to 'services/incremental/IncrementalService.cpp')
-rw-r--r-- | services/incremental/IncrementalService.cpp | 19 |
1 files changed, 15 insertions, 4 deletions
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp index 886c1e5b98ea..2fa927bcccfd 100644 --- a/services/incremental/IncrementalService.cpp +++ b/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, |