From 7f05f6a566d59d1dae40f88e1d0f82bf29452bd1 Mon Sep 17 00:00:00 2001 From: Gavin Corkery Date: Tue, 7 Apr 2020 00:12:15 +0100 Subject: [PATCH] Call startObservingHealth on handler thread Both RescueParty and PackageWatchdog receive DeviceConfig callbacks. RescueParty may act on this callback by calling startObservingHealth, while PackageWatchdog will call updateConfigs. There may be a contention for PackageWatchdog#mLock in this case. Move some of the logic in startObservingHealth to the long task handler thread to avoid this. Bug: 150114865 Test: atest StagedRollbackTest Test: atest RescuePartyTest Change-Id: I74e1b21b7adddf35759e70211c47d31acfcae7f2 --- .../com/android/server/PackageWatchdog.java | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java index d9e7c38519063..e77458cc955a1 100644 --- a/services/core/java/com/android/server/PackageWatchdog.java +++ b/services/core/java/com/android/server/PackageWatchdog.java @@ -315,28 +315,31 @@ public class PackageWatchdog { // causing any elapsed time to be deducted from all existing packages before we add new // packages. This maintains the invariant that the elapsed time for ALL (new and existing) // packages is the same. - syncState("observing new packages"); + mLongTaskHandler.post(() -> { + syncState("observing new packages"); - synchronized (mLock) { - ObserverInternal oldObserver = mAllObservers.get(observer.getName()); - if (oldObserver == null) { - Slog.d(TAG, observer.getName() + " started monitoring health " - + "of packages " + packageNames); - mAllObservers.put(observer.getName(), - new ObserverInternal(observer.getName(), packages)); - } else { - Slog.d(TAG, observer.getName() + " added the following " - + "packages to monitor " + packageNames); - oldObserver.updatePackagesLocked(packages); + synchronized (mLock) { + ObserverInternal oldObserver = mAllObservers.get(observer.getName()); + if (oldObserver == null) { + Slog.d(TAG, observer.getName() + " started monitoring health " + + "of packages " + packageNames); + mAllObservers.put(observer.getName(), + new ObserverInternal(observer.getName(), packages)); + } else { + Slog.d(TAG, observer.getName() + " added the following " + + "packages to monitor " + packageNames); + oldObserver.updatePackagesLocked(packages); + } } - } - // Register observer in case not already registered - registerHealthObserver(observer); + // Register observer in case not already registered + registerHealthObserver(observer); + + // Sync after we add the new packages to the observers. We may have received packges + // requiring an earlier schedule than we are currently scheduled for. + syncState("updated observers"); + }); - // Sync after we add the new packages to the observers. We may have received packges - // requiring an earlier schedule than we are currently scheduled for. - syncState("updated observers"); } /**