DO NOT MERGE Add stop method to backup handler thread.

am: 842369a82b

Change-Id: I42795aea1d5f31f04e65a66a8e78ce2431d90d3c

services/backup/java/com/android/server/backup/UserBackupManagerService.java

@@ -166,6 +166,52 @@ 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 +298,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 +308,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 +532,7 @@ public class UserBackupManagerService {
         mAgentTimeoutParameters.start();
 
         checkNotNull(userBackupThread, "userBackupThread cannot be null");
-        mUserBackupThread = userBackupThread;
+        mBackupHandler = new BackupHandler(this, userBackupThread);
-        mBackupHandler = new BackupHandler(this, userBackupThread.getLooper());
 
         // Set up our bookkeeping
         final ContentResolver resolver = context.getContentResolver();
@@ -588,7 +632,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 +655,7 @@ public class UserBackupManagerService {
         mContext.unregisterReceiver(mRunBackupReceiver);
         mContext.unregisterReceiver(mRunInitReceiver);
         mContext.unregisterReceiver(mPackageTrackingReceiver);
-        mUserBackupThread.quit();
+        mBackupHandler.stop();
     }
 
     public @UserIdInt int getUserId() {
@@ -668,7 +715,7 @@ public class UserBackupManagerService {
         mSetupComplete = setupComplete;
     }
 
-    public PowerManager.WakeLock getWakelock() {
+    public BackupWakeLock getWakelock() {
         return mWakelock;
     }
 
@@ -679,7 +726,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() {

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) {
+    private final HandlerThread mBackupThread;
-        super(looper);
+    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) {

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();
 

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);

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;

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);
 

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*"));
     }
 
     /**