Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6118857c authored by Paul Hadfield's avatar Paul Hadfield
Browse files

Trigger unbindBackupAgent if app dies during backup

This is to fix a bug where, if an app crashes in restricted mode,
when the user restarts the app, it would be launched still in
restricted mode. That happens because ActivityManager still has
a BackupRecord indicating that the app should be in restricted
mode, so next time the app launches, attachApplicationLocked()
calls IApplicationThread.bindApplication() with the restricted
mode flag set.

To avoid this we must complete the clean up actions done when
backup agents report errors. UserBackupManagerService has a
call-back method agentDisconnected() that is notified if an
agent dies. In that method we call handleCancel() on all
outstanding operations that are linked to the package name,
according to OperationStorage.  The result is a call to the
Activity Manager's unbindBackupAgent() which then removes the
obsolete BackupRecord.

Two new tests for agentDisconnected() verify that appropriate
calls are made to OperationStore.

BUG: 161089758
Test: atest FrameworksServicesTests:com.android.server.backup
      atest BackupFrameworksServicesRoboTests
      atest CtsBackupHostTestCases
      atest CtsBackupTestCases
      atest GtsBackupTestCase
      Also, manual testing with an app that deliberately dies
      during full backup: the lifecycle operation storage is
      used to cancel the backup; the app restarts in normal
      and not restricted mode.
Change-Id: Ic4cf3875d59e3580b026340e884093f3aa1b09b9
parent 789acb12
Loading
Loading
Loading
Loading
+14 −1
Original line number Diff line number Diff line
@@ -3718,7 +3718,6 @@ public class UserBackupManagerService {
     * only be called from the {@link ActivityManager}.
     */
    public void agentDisconnected(String packageName) {
        // TODO: handle backup being interrupted
        synchronized (mAgentConnectLock) {
            if (Binder.getCallingUid() == Process.SYSTEM_UID) {
                mConnectedAgent = null;
@@ -3732,6 +3731,20 @@ public class UserBackupManagerService {
                                        + Binder.getCallingUid()
                                        + " claiming agent disconnected"));
            }
            Slog.w(TAG, "agentDisconnected: the backup agent for " + packageName
                    + " died: cancel current operations");

            // handleCancel() causes the PerformFullTransportBackupTask to go on to
            // tearDownAgentAndKill: that will unbindBackupAgent in the Activity Manager, so
            // that the package being backed up doesn't get stuck in restricted mode until the
            // backup time-out elapses.
            for (int token : mOperationStorage.operationTokensForPackage(packageName)) {
                if (MORE_DEBUG) {
                    Slog.d(TAG, "agentDisconnected: will handleCancel(all) for token:"
                            + Integer.toHexString(token));
                }
                handleCancel(token, true /* cancelAll */);
            }
            mAgentConnectLock.notifyAll();
        }
    }
+15 −9
Original line number Diff line number Diff line
@@ -56,12 +56,15 @@ import com.android.server.backup.utils.BackupEligibilityRules;
import com.android.server.backup.utils.BackupManagerMonitorUtils;
import com.android.server.backup.utils.BackupObserverUtils;

import com.google.android.collect.Sets;

import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
@@ -195,8 +198,6 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba
            return;
        }

        registerTask();

        for (String pkg : whichPackages) {
            try {
                PackageManager pm = backupManagerService.getPackageManager();
@@ -262,11 +263,15 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba
        }

        mPackages = backupManagerService.filterUserFacingPackages(mPackages);

        Set<String> packageNames = Sets.newHashSet();
        for (PackageInfo pkgInfo : mPackages) {
            packageNames.add(pkgInfo.packageName);
        }

    private void registerTask() {
        Slog.d(TAG, "backupmanager pftbt token=" + Integer.toHexString(mCurrentOpToken));
        mOperationStorage.registerOperation(mCurrentOpToken, OpState.PENDING, this, OpType.BACKUP);
        mOperationStorage.registerOperationForPackages(mCurrentOpToken, OpState.PENDING,
                packageNames, this, OpType.BACKUP);
    }

    // public, because called from KeyValueBackupTask.finishTask.
@@ -834,12 +839,13 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba
            mBackupResult = BackupTransport.AGENT_ERROR;
            mQuota = quota;
            mTransportFlags = transportFlags;
            registerTask();
            registerTask(target.packageName);
        }

        void registerTask() {
            mOperationStorage.registerOperation(mCurrentOpToken,
                    OpState.PENDING, this, OpType.BACKUP_WAIT);
        void registerTask(String packageName) {
            Set<String> packages = Sets.newHashSet(packageName);
            mOperationStorage.registerOperationForPackages(mCurrentOpToken, OpState.PENDING,
                    packages, this, OpType.BACKUP_WAIT);
        }

        void unregisterTask() {
+33 −0
Original line number Diff line number Diff line
@@ -19,8 +19,12 @@ package com.android.server.backup;
import static com.google.common.truth.Truth.assertThat;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.app.backup.BackupAgent;
@@ -42,12 +46,16 @@ import com.android.server.backup.transport.BackupTransportClient;
import com.android.server.backup.transport.TransportConnection;
import com.android.server.backup.utils.BackupEligibilityRules;

import com.google.common.collect.ImmutableSet;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import java.util.function.IntConsumer;

@Presubmit
@RunWith(AndroidJUnit4.class)
public class UserBackupManagerServiceTest {
@@ -163,6 +171,31 @@ public class UserBackupManagerServiceTest {
        assertThat(operationType).isEqualTo(OperationType.MIGRATION);
    }

    @Test
    public void testAgentDisconnected_cancelsCurrentOperations() throws Exception {
        when(mOperationStorage.operationTokensForPackage(eq("com.android.foo"))).thenReturn(
                ImmutableSet.of(123, 456, 789)
        );

        mService.agentDisconnected("com.android.foo");

        verify(mOperationStorage).cancelOperation(eq(123), eq(true), any(IntConsumer.class));
        verify(mOperationStorage).cancelOperation(eq(456), eq(true), any());
        verify(mOperationStorage).cancelOperation(eq(789), eq(true), any());
    }

    @Test
    public void testAgentDisconnected_unknownPackageName_cancelsNothing() throws Exception {
        when(mOperationStorage.operationTokensForPackage(eq("com.android.foo"))).thenReturn(
                ImmutableSet.of()
        );

        mService.agentDisconnected("com.android.foo");

        verify(mOperationStorage, never())
                .cancelOperation(anyInt(), anyBoolean(), any(IntConsumer.class));
    }

    private static PackageInfo getPackageInfo(String packageName) {
        PackageInfo packageInfo = new PackageInfo();
        packageInfo.applicationInfo = new ApplicationInfo();