diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index ed4e59611c229..d599aabbaa287 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -166,6 +166,53 @@ import java.util.concurrent.atomic.AtomicInteger; /** System service that performs backup/restore operations. */ public class UserBackupManagerService { + /** + * Wrapper over {@link PowerManager.WakeLock} to prevent double-free exceptions on release() + * after quit(). + */ + public static class BackupWakeLock { + private final PowerManager.WakeLock mPowerManagerWakeLock; + private boolean mHasQuit = false; + + public BackupWakeLock(PowerManager.WakeLock powerManagerWakeLock) { + mPowerManagerWakeLock = powerManagerWakeLock; + } + + /** Acquires the {@link PowerManager.WakeLock} if hasn't been quit. */ + public synchronized void acquire() { + if (mHasQuit) { + Slog.v(TAG, "Ignore wakelock acquire after quit: " + mPowerManagerWakeLock.getTag()); + return; + } + mPowerManagerWakeLock.acquire(); + } + + /** Releases the {@link PowerManager.WakeLock} if hasn't been quit. */ + public synchronized void release() { + if (mHasQuit) { + Slog.v(TAG, "Ignore wakelock release after quit: " + mPowerManagerWakeLock.getTag()); + return; + } + mPowerManagerWakeLock.release(); + } + + /** + * Returns true if the {@link PowerManager.WakeLock} has been acquired but not yet released. + */ + public synchronized boolean isHeld() { + return mPowerManagerWakeLock.isHeld(); + } + + /** Release the {@link PowerManager.WakeLock} till it isn't held. */ + public synchronized void quit() { + while (mPowerManagerWakeLock.isHeld()) { + Slog.v(TAG, "Releasing wakelock: " + mPowerManagerWakeLock.getTag()); + mPowerManagerWakeLock.release(); + } + mHasQuit = true; + } + } + // Persistently track the need to do a full init. private static final String INIT_SENTINEL_FILE_NAME = "_need_init_"; @@ -252,7 +299,6 @@ public class UserBackupManagerService { private final @UserIdInt int mUserId; private final BackupAgentTimeoutParameters mAgentTimeoutParameters; private final TransportManager mTransportManager; - private final HandlerThread mUserBackupThread; private final Context mContext; private final PackageManager mPackageManager; @@ -263,7 +309,7 @@ public class UserBackupManagerService { private final AlarmManager mAlarmManager; private final IStorageManager mStorageManager; private final BackupManagerConstants mConstants; - private final PowerManager.WakeLock mWakelock; + private final BackupWakeLock mWakelock; private final BackupHandler mBackupHandler; private final IBackupManager mBackupManagerBinder; @@ -487,8 +533,7 @@ public class UserBackupManagerService { mAgentTimeoutParameters.start(); checkNotNull(userBackupThread, "userBackupThread cannot be null"); - mUserBackupThread = userBackupThread; - mBackupHandler = new BackupHandler(this, userBackupThread.getLooper()); + mBackupHandler = new BackupHandler(this, userBackupThread); // Set up our bookkeeping final ContentResolver resolver = context.getContentResolver(); @@ -588,7 +633,10 @@ public class UserBackupManagerService { mBackupHandler.postDelayed(this::parseLeftoverJournals, INITIALIZATION_DELAY_MILLIS); // Power management - mWakelock = mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "*backup*-" + userId); + mWakelock = new BackupWakeLock( + mPowerManager.newWakeLock( + PowerManager.PARTIAL_WAKE_LOCK, + "*backup*-" + userId + "-" + userBackupThread.getThreadId())); // Set up the various sorts of package tracking we do mFullBackupScheduleFile = new File(mBaseStateDir, "fb-schedule"); @@ -608,7 +656,7 @@ public class UserBackupManagerService { mContext.unregisterReceiver(mRunBackupReceiver); mContext.unregisterReceiver(mRunInitReceiver); mContext.unregisterReceiver(mPackageTrackingReceiver); - mUserBackupThread.quit(); + mBackupHandler.stop(); } public @UserIdInt int getUserId() { @@ -668,7 +716,7 @@ public class UserBackupManagerService { mSetupComplete = setupComplete; } - public PowerManager.WakeLock getWakelock() { + public BackupWakeLock getWakelock() { return mWakelock; } @@ -679,7 +727,7 @@ public class UserBackupManagerService { @VisibleForTesting public void setWorkSource(@Nullable WorkSource workSource) { // TODO: This is for testing, unfortunately WakeLock is final and WorkSource is not exposed - mWakelock.setWorkSource(workSource); + mWakelock.mPowerManagerWakeLock.setWorkSource(workSource); } public Handler getBackupHandler() { diff --git a/services/backup/java/com/android/server/backup/internal/BackupHandler.java b/services/backup/java/com/android/server/backup/internal/BackupHandler.java index ba153bf90ebea..059b1b9379afb 100644 --- a/services/backup/java/com/android/server/backup/internal/BackupHandler.java +++ b/services/backup/java/com/android/server/backup/internal/BackupHandler.java @@ -23,7 +23,7 @@ import static com.android.server.backup.BackupManagerService.TAG; import android.app.backup.RestoreSet; import android.content.Intent; import android.os.Handler; -import android.os.Looper; +import android.os.HandlerThread; import android.os.Message; import android.os.RemoteException; import android.os.UserHandle; @@ -83,19 +83,47 @@ public class BackupHandler extends Handler { // backup task state machine tick public static final int MSG_BACKUP_RESTORE_STEP = 20; public static final int MSG_OP_COMPLETE = 21; + // Release the wakelock. This is used to ensure we don't hold it after + // a user is removed. This will also terminate the looper thread. + public static final int MSG_STOP = 22; private final UserBackupManagerService backupManagerService; private final BackupAgentTimeoutParameters mAgentTimeoutParameters; - public BackupHandler(UserBackupManagerService backupManagerService, Looper looper) { - super(looper); + private final HandlerThread mBackupThread; + private volatile boolean mIsStopping = false; + + public BackupHandler( + UserBackupManagerService backupManagerService, HandlerThread backupThread) { + super(backupThread.getLooper()); + mBackupThread = backupThread; this.backupManagerService = backupManagerService; mAgentTimeoutParameters = Preconditions.checkNotNull( backupManagerService.getAgentTimeoutParameters(), "Timeout parameters cannot be null"); } + /** + * Put the BackupHandler into a stopping state where the remaining messages on the queue will be + * silently dropped and the {@link WakeLock} held by the {@link UserBackupManagerService} will + * then be released. + */ + public void stop() { + mIsStopping = true; + sendMessage(obtainMessage(BackupHandler.MSG_STOP)); + } + public void handleMessage(Message msg) { + if (msg.what == MSG_STOP) { + Slog.v(TAG, "Stopping backup handler"); + backupManagerService.getWakelock().quit(); + mBackupThread.quitSafely(); + } + + if (mIsStopping) { + // If we're finishing all other types of messages should be ignored + return; + } TransportManager transportManager = backupManagerService.getTransportManager(); switch (msg.what) { diff --git a/services/backup/java/com/android/server/backup/internal/RunInitializeReceiver.java b/services/backup/java/com/android/server/backup/internal/RunInitializeReceiver.java index 97711e3c27edf..96d61e5755c89 100644 --- a/services/backup/java/com/android/server/backup/internal/RunInitializeReceiver.java +++ b/services/backup/java/com/android/server/backup/internal/RunInitializeReceiver.java @@ -23,7 +23,6 @@ import static com.android.server.backup.UserBackupManagerService.RUN_INITIALIZE_ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; -import android.os.PowerManager; import android.util.Slog; import com.android.server.backup.UserBackupManagerService; @@ -57,7 +56,8 @@ public class RunInitializeReceiver extends BroadcastReceiver { mUserBackupManagerService.clearPendingInits(); - PowerManager.WakeLock wakelock = mUserBackupManagerService.getWakelock(); + UserBackupManagerService.BackupWakeLock wakelock = + mUserBackupManagerService.getWakelock(); wakelock.acquire(); OnTaskFinishedListener listener = caller -> wakelock.release(); diff --git a/services/backup/java/com/android/server/backup/restore/ActiveRestoreSession.java b/services/backup/java/com/android/server/backup/restore/ActiveRestoreSession.java index 10304c39f0213..5a57cdc39402a 100644 --- a/services/backup/java/com/android/server/backup/restore/ActiveRestoreSession.java +++ b/services/backup/java/com/android/server/backup/restore/ActiveRestoreSession.java @@ -34,7 +34,6 @@ import android.content.pm.PackageManager.NameNotFoundException; import android.os.Binder; import android.os.Handler; import android.os.Message; -import android.os.PowerManager; import android.util.Slog; import com.android.server.backup.TransportManager; @@ -110,7 +109,7 @@ public class ActiveRestoreSession extends IRestoreSession.Stub { // comes in. mBackupManagerService.getBackupHandler().removeMessages(MSG_RESTORE_SESSION_TIMEOUT); - PowerManager.WakeLock wakelock = mBackupManagerService.getWakelock(); + UserBackupManagerService.BackupWakeLock wakelock = mBackupManagerService.getWakelock(); wakelock.acquire(); // Prevent lambda from leaking 'this' @@ -392,7 +391,7 @@ public class ActiveRestoreSession extends IRestoreSession.Stub { Handler backupHandler = mBackupManagerService.getBackupHandler(); backupHandler.removeMessages(MSG_RESTORE_SESSION_TIMEOUT); - PowerManager.WakeLock wakelock = mBackupManagerService.getWakelock(); + UserBackupManagerService.BackupWakeLock wakelock = mBackupManagerService.getWakelock(); wakelock.acquire(); if (MORE_DEBUG) { Slog.d(TAG, callerLogString); diff --git a/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java b/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java index 43691e0e61637..4aba1f487f49c 100644 --- a/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java +++ b/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java @@ -94,7 +94,6 @@ import android.os.Handler; import android.os.Looper; import android.os.Message; import android.os.ParcelFileDescriptor; -import android.os.PowerManager; import android.os.RemoteException; import android.platform.test.annotations.Presubmit; import android.util.Pair; @@ -184,7 +183,7 @@ public class KeyValueBackupTaskTest { private TransportData mTransport; private ShadowLooper mShadowBackupLooper; private Handler mBackupHandler; - private PowerManager.WakeLock mWakeLock; + private UserBackupManagerService.BackupWakeLock mWakeLock; private KeyValueBackupReporter mReporter; private PackageManager mPackageManager; private ShadowPackageManager mShadowPackageManager; diff --git a/services/robotests/backup/src/com/android/server/backup/restore/ActiveRestoreSessionTest.java b/services/robotests/backup/src/com/android/server/backup/restore/ActiveRestoreSessionTest.java index f4cea7ae86a60..3fc421dfb6e97 100644 --- a/services/robotests/backup/src/com/android/server/backup/restore/ActiveRestoreSessionTest.java +++ b/services/robotests/backup/src/com/android/server/backup/restore/ActiveRestoreSessionTest.java @@ -18,7 +18,7 @@ package com.android.server.backup.restore; import static com.android.server.backup.testing.BackupManagerServiceTestUtils.createBackupWakeLock; import static com.android.server.backup.testing.BackupManagerServiceTestUtils.setUpBackupManagerServiceBasics; -import static com.android.server.backup.testing.BackupManagerServiceTestUtils.startBackupThreadAndGetLooper; +import static com.android.server.backup.testing.BackupManagerServiceTestUtils.startBackupThread; import static com.android.server.backup.testing.TestUtils.assertEventLogged; import static com.android.server.backup.testing.TestUtils.assertEventNotLogged; import static com.android.server.backup.testing.TransportData.backupTransport; @@ -44,8 +44,8 @@ import android.app.backup.RestoreSet; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.os.Handler; +import android.os.HandlerThread; import android.os.Looper; -import android.os.PowerManager; import android.os.RemoteException; import android.platform.test.annotations.Presubmit; @@ -98,7 +98,7 @@ public class ActiveRestoreSessionTest { @Mock private IBackupManagerMonitor mMonitor; private ShadowLooper mShadowBackupLooper; private ShadowApplication mShadowApplication; - private PowerManager.WakeLock mWakeLock; + private UserBackupManagerService.BackupWakeLock mWakeLock; private TransportData mTransport; private RestoreSet mRestoreSet1; private RestoreSet mRestoreSet2; @@ -118,7 +118,8 @@ public class ActiveRestoreSessionTest { mShadowPackageManager = shadowOf(application.getPackageManager()); - Looper backupLooper = startBackupThreadAndGetLooper(); + HandlerThread handlerThread = startBackupThread(null); + Looper backupLooper = handlerThread.getLooper(); mShadowBackupLooper = shadowOf(backupLooper); Handler mainHandler = new Handler(Looper.getMainLooper()); @@ -129,7 +130,7 @@ public class ActiveRestoreSessionTest { // We need to mock BMS timeout parameters before initializing the BackupHandler since // the constructor of BackupHandler relies on it. when(mBackupManagerService.getAgentTimeoutParameters()).thenReturn(agentTimeoutParameters); - BackupHandler backupHandler = new BackupHandler(mBackupManagerService, backupLooper); + BackupHandler backupHandler = new BackupHandler(mBackupManagerService, handlerThread); mWakeLock = createBackupWakeLock(application); diff --git a/services/robotests/backup/src/com/android/server/backup/testing/BackupManagerServiceTestUtils.java b/services/robotests/backup/src/com/android/server/backup/testing/BackupManagerServiceTestUtils.java index 47abcc5a2dd15..392d182328a54 100644 --- a/services/robotests/backup/src/com/android/server/backup/testing/BackupManagerServiceTestUtils.java +++ b/services/robotests/backup/src/com/android/server/backup/testing/BackupManagerServiceTestUtils.java @@ -113,7 +113,7 @@ public class BackupManagerServiceTestUtils { TransportManager transportManager, PackageManager packageManager, Handler backupHandler, - PowerManager.WakeLock wakeLock, + UserBackupManagerService.BackupWakeLock wakeLock, BackupAgentTimeoutParameters agentTimeoutParameters) { when(backupManagerService.getContext()).thenReturn(application); @@ -161,10 +161,12 @@ public class BackupManagerServiceTestUtils { }); } - public static PowerManager.WakeLock createBackupWakeLock(Application application) { + public static UserBackupManagerService.BackupWakeLock createBackupWakeLock( + Application application) { PowerManager powerManager = (PowerManager) application.getSystemService(Context.POWER_SERVICE); - return powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "*backup*"); + return new UserBackupManagerService.BackupWakeLock( + powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "*backup*")); } /**