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

Commit d2ecdf18 authored by Sarp Misoglu's avatar Sarp Misoglu
Browse files

Don't cancel the entire backup if agent disconnects and log if the agent pipe is broken 

Normally the bug fix and the extra logging should be in separate CLs.
However as explained in the linked bug, there is a race condition
between the method throwing the IOException returning and us getting a
cancellation callback from ActivityManager. If we get the callback
first, the session fails. The extra IPC from the logging in handling the
exception means the callback gets a few additional milliseconds to win
the race. So with the logging, the race condition is much more likely to
happen and the logging should not be checked in without the bug fix. And
if one gets rolled back, the other should as well. This is why I made
them a single CL.

Flag: EXEMPT bugfix
Bug: 400669414
Test: manual // PFTBT is notoriously impossible to write unit tests for.
Change-Id: Icbcb72a876f927a94675c9837b0206f1a631dff5
parent 7aec8c1a

cmds/bmgr/src/com/android/commands/bmgr/Bmgr.java

+2 −0
Original line number Diff line number Diff line
@@ -1309,6 +1309,8 @@ public class Bmgr { 
                return "AGENT_FAILURE_DURING_RESTORE";

            case BackupManagerMonitor.LOG_EVENT_ID_FAILED_TO_READ_DATA_FROM_TRANSPORT:

                return "FAILED_TO_READ_DATA_FROM_TRANSPORT";

            case BackupManagerMonitor.LOG_EVENT_ID_FULL_BACKUP_AGENT_PIPE_BROKEN:

                return "LOG_EVENT_ID_FULL_BACKUP_AGENT_PIPE_BROKEN";

            default:

                return "UNKNOWN_ID";

        }

core/java/android/app/backup/BackupManagerMonitor.java

+3 −0
Original line number Diff line number Diff line
@@ -297,6 +297,9 @@ public class BackupManagerMonitor { 
   @hide */

  public static final int LOG_EVENT_ID_FAILED_TO_READ_DATA_FROM_TRANSPORT = 81;



  /** The pipe between the BackupAgent and the framework was broken during full backup. @hide */

  public static final int LOG_EVENT_ID_FULL_BACKUP_AGENT_PIPE_BROKEN = 82;



  /**

   * This method will be called each time something important happens on BackupManager.

   *

services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java

+7 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import static com.android.server.backup.UserBackupManagerService.SHARED_BACKUP_A 
import android.annotation.UserIdInt;

import android.app.ApplicationThreadConstants;

import android.app.IBackupAgent;

import android.app.backup.BackupManagerMonitor;

import android.app.backup.BackupTransport;

import android.app.backup.FullBackupDataOutput;

import android.content.pm.ApplicationInfo;
@@ -268,6 +269,12 @@ public class FullBackupEngine { 
                mBackupManagerMonitorEventSender.monitorAgentLoggingResults(mPkg, mAgent);

            } catch (IOException e) {

                Slog.e(TAG, "Error backing up " + mPkg.packageName + ": " + e.getMessage());

                // This is likely due to the app process dying.

                mBackupManagerMonitorEventSender.monitorEvent(

                        BackupManagerMonitor.LOG_EVENT_ID_FULL_BACKUP_AGENT_PIPE_BROKEN,

                        mPkg,

                        BackupManagerMonitor.LOG_EVENT_CATEGORY_AGENT,

                        /* extras= */ null);

                result = BackupTransport.AGENT_ERROR;

            } finally {

                try {

services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java

+8 −0
Original line number Diff line number Diff line
@@ -294,6 +294,14 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba 
            // SinglePackageBackupPreflight.

            if (cancellationReason == CancellationReason.TIMEOUT) {

                Slog.wtf(TAG, "This task cannot time out");

                return;

            }



            // We don't cancel the entire operation if a single agent is disconnected unexpectedly.

            // SinglePackageBackupRunner and SinglePackageBackupPreflight will receive the same

            // callback and fail gracefully. The operation should then continue to the next package.

            if (cancellationReason == CancellationReason.AGENT_DISCONNECTED) {

                return;

            }



            if (mCancelled) {

services/backup/java/com/android/server/backup/utils/BackupManagerMonitorDumpsysUtils.java

+2 −0
Original line number Diff line number Diff line
@@ -389,6 +389,8 @@ public class BackupManagerMonitorDumpsysUtils { 
                    "Agent failure during restore";

            case BackupManagerMonitor.LOG_EVENT_ID_FAILED_TO_READ_DATA_FROM_TRANSPORT ->

                    "Failed to read data from Transport";

            case BackupManagerMonitor.LOG_EVENT_ID_FULL_BACKUP_AGENT_PIPE_BROKEN ->

                "LOG_EVENT_ID_FULL_BACKUP_AGENT_PIPE_BROKEN";

            default -> "Unknown log event ID: " + code;

        };

        return id;