Handle uncaught exceptions in BackupHandler
Bug: 144431410
Test: 1. atest BackupHandlerTest
2. Manual (with and without the fix):
1) Locally create a host-side CTS test that extends
BaseMultiUserBackupHostSideTest
2) Modify the test so that it creates a user, starts backup
init and removes the user
3) Add log in BackupHandler to indicate when an exception is
suppressed.
3) If the fix is applied, verify the crash doesn't happen and
the log message from 3) is present. If the fix isn't applied
verify that the crash happens.
After backup service for a user is stopped, leftover work on the
corresponding BackupHandler can throw exceptions. If uncaught, they
can crash the system process. Catch all uncaught BackupHandler
exceptions after the backup service has entered the stopping state, to
allow any leftover work to finish harmlessly.
Change-Id: I8c233ad0e117ec0ae65599a762d87f15f8a3cec2
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 0964b31..eb62620 100644
--- a/services/backup/java/com/android/server/backup/internal/BackupHandler.java
+++ b/services/backup/java/com/android/server/backup/internal/BackupHandler.java
@@ -31,6 +31,7 @@
import android.util.Pair;
import android.util.Slog;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.backup.IBackupTransport;
import com.android.server.EventLogTags;
import com.android.server.backup.BackupAgentTimeoutParameters;
@@ -91,7 +92,9 @@
private final BackupAgentTimeoutParameters mAgentTimeoutParameters;
private final HandlerThread mBackupThread;
- private volatile boolean mIsStopping = false;
+
+ @VisibleForTesting
+ volatile boolean mIsStopping = false;
public BackupHandler(
UserBackupManagerService backupManagerService, HandlerThread backupThread) {
@@ -113,6 +116,24 @@
sendMessage(obtainMessage(BackupHandler.MSG_STOP));
}
+ @Override
+ public void dispatchMessage(Message message) {
+ try {
+ dispatchMessageInternal(message);
+ } catch (Exception e) {
+ // If the backup service is stopping, we'll suppress all exceptions to avoid crashes
+ // caused by code still running after the current user has become unavailable.
+ if (!mIsStopping) {
+ throw e;
+ }
+ }
+ }
+
+ @VisibleForTesting
+ void dispatchMessageInternal(Message message) {
+ super.dispatchMessage(message);
+ }
+
public void handleMessage(Message msg) {
if (msg.what == MSG_STOP) {
Slog.v(TAG, "Stopping backup handler");
@@ -414,7 +435,7 @@
try {
params.observer.onTimeout();
} catch (RemoteException e) {
- /* don't care if the app has gone away */
+ /* don't care if the app has gone away */
}
}
} else {
diff --git a/services/tests/servicestests/src/com/android/server/backup/internal/BackupHandlerTest.java b/services/tests/servicestests/src/com/android/server/backup/internal/BackupHandlerTest.java
new file mode 100644
index 0000000..fa35e3f
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/backup/internal/BackupHandlerTest.java
@@ -0,0 +1,139 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.backup.internal;
+
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertThrows;
+import static org.testng.Assert.assertTrue;
+
+import android.os.HandlerThread;
+import android.os.Message;
+import android.platform.test.annotations.Presubmit;
+
+import androidx.test.runner.AndroidJUnit4;
+
+import com.android.server.backup.BackupAgentTimeoutParameters;
+import com.android.server.backup.UserBackupManagerService;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+@Presubmit
+@RunWith(AndroidJUnit4.class)
+public class BackupHandlerTest {
+ private static final int MESSAGE_TIMEOUT_MINUTES = 1;
+
+ @Mock private UserBackupManagerService mUserBackupManagerService;
+ @Mock private BackupAgentTimeoutParameters mTimeoutParameters;
+
+ private HandlerThread mHandlerThread;
+ private CountDownLatch mCountDownLatch;
+ private boolean mExceptionPropagated;
+
+ @Before
+ public void setUp() {
+ MockitoAnnotations.initMocks(/* testClass */ this);
+ when(mUserBackupManagerService.getAgentTimeoutParameters()).thenReturn(mTimeoutParameters);
+
+ mExceptionPropagated = false;
+ mCountDownLatch = new CountDownLatch(/* count */ 1);
+ mHandlerThread = new HandlerThread("BackupHandlerTestThread");
+ mHandlerThread.start();
+ }
+
+ @After
+ public void tearDown() {
+ mHandlerThread.quit();
+ }
+
+ @Test
+ public void testSendMessage_propagatesExceptions() throws Exception {
+ BackupHandler handler = new TestBackupHandler(/* shouldStop */ false);
+ handler.sendMessage(getMessage());
+ mCountDownLatch.await(MESSAGE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
+
+ assertTrue(mExceptionPropagated);
+ }
+
+ @Test
+ public void testPost_propagatesExceptions() throws Exception {
+ BackupHandler handler = new TestBackupHandler(/* shouldStop */ false);
+ handler.post(() -> {});
+ mCountDownLatch.await(MESSAGE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
+
+ assertTrue(mExceptionPropagated);
+ }
+
+ @Test
+ public void testSendMessage_stopping_doesntPropagateExceptions() throws Exception {
+ BackupHandler handler = new TestBackupHandler(/* shouldStop */ true);
+ handler.sendMessage(getMessage());
+ mCountDownLatch.await(MESSAGE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
+
+ assertFalse(mExceptionPropagated);
+ }
+
+ @Test
+ public void testPost_stopping_doesntPropagateExceptions() throws Exception {
+ BackupHandler handler = new TestBackupHandler(/* shouldStop */ true);
+ handler.post(() -> {});
+ mCountDownLatch.await(MESSAGE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
+
+ assertFalse(mExceptionPropagated);
+ }
+
+ private static Message getMessage() {
+ Message message = Message.obtain();
+ message.what = -1;
+ return message;
+ }
+
+ private class TestBackupHandler extends BackupHandler {
+ private final boolean mShouldStop;
+
+ TestBackupHandler(boolean shouldStop) {
+ super(mUserBackupManagerService, mHandlerThread);
+
+ mShouldStop = shouldStop;
+ }
+
+ @Override
+ public void dispatchMessage(Message msg) {
+ try {
+ super.dispatchMessage(msg);
+ } catch (Exception e) {
+ mExceptionPropagated = true;
+ } finally {
+ mCountDownLatch.countDown();
+ }
+ }
+
+ @Override
+ void dispatchMessageInternal(Message msg) {
+ mIsStopping = mShouldStop;
+ throw new RuntimeException();
+ }
+ }
+}