From b6dde00d038b899e5235b6a12d2ec6a2864f2022 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Fri, 13 Nov 2020 17:32:05 +0000 Subject: [PATCH 1/2] Clean up staged session data on validation failure Bug: 173132101 Test: atest StagedInstallInternalTest#testStagedInstallationShouldCleanUpOnValidationFailure Test: atest StagedInstallInternalTest#testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage Change-Id: Idd6597cd0d2dda34a8a7626b585401eeee39c31f --- .../server/pm/PackageInstallerSession.java | 4 ++++ .../StagedInstallInternalTest.java | 13 +++++++++++ .../host/StagedInstallInternalTest.java | 23 ++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 5d2928e1a854b..f4ad38d33172c 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -1676,6 +1676,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { destroyInternal(); // Dispatch message to remove session from PackageInstallerService. dispatchSessionFinished(error, detailMessage, null); + // TODO(b/173194203): clean up staged session in destroyInternal() call instead + if (isStaged() && stageDir != null) { + cleanStageDir(); + } } private void onSessionVerificationFailure(int error, String msg) { diff --git a/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java b/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java index e5411dee7805a..2a601e5cb86dc 100644 --- a/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java +++ b/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java @@ -127,6 +127,19 @@ public class StagedInstallInternalTest { InstallUtils.getPackageInstaller().abandonSession(id4); } + @Test + public void testStagedInstallationShouldCleanUpOnValidationFailure() throws Exception { + InstallUtils.commitExpectingFailure(AssertionError.class, "INSTALL_FAILED_INVALID_APK", + Install.single(TestApp.AIncompleteSplit).setStaged()); + } + + @Test + public void testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage() + throws Exception { + InstallUtils.commitExpectingFailure(AssertionError.class, "INSTALL_FAILED_INVALID_APK", + Install.multi(TestApp.AIncompleteSplit, TestApp.B1, TestApp.Apex1).setStaged()); + } + private static void assertSessionReady(int sessionId) { assertSessionState(sessionId, (session) -> assertThat(session.isStagedSessionReady()).isTrue()); diff --git a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java index 702f8719ff243..1faa4691d13eb 100644 --- a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java +++ b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java @@ -30,6 +30,7 @@ import com.android.compatibility.common.tradefed.build.CompatibilityBuildHelper; import com.android.ddmlib.Log; import com.android.tests.rollback.host.AbandonSessionsRule; import com.android.tests.util.ModuleTestUtils; +import com.android.tradefed.device.DeviceNotAvailableException; import com.android.tradefed.testtype.DeviceJUnit4ClassRunner; import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test; import com.android.tradefed.util.CommandResult; @@ -250,9 +251,27 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test { assertThat(after).isEqualTo(before); } - private List getStagingDirectories() { + @Test + public void testStagedInstallationShouldCleanUpOnValidationFailure() throws Exception { + List before = getStagingDirectories(); + runPhase("testStagedInstallationShouldCleanUpOnValidationFailure"); + List after = getStagingDirectories(); + assertThat(after).isEqualTo(before); + } + + @Test + public void testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage() + throws Exception { + List before = getStagingDirectories(); + runPhase("testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage"); + List after = getStagingDirectories(); + assertThat(after).isEqualTo(before); + } + + private List getStagingDirectories() throws DeviceNotAvailableException { String baseDir = "/data/app-staging"; try { + getDevice().enableAdbRoot(); return getDevice().getFileEntry(baseDir).getChildren(false) .stream().filter(entry -> entry.getName().matches("session_\\d+")) .map(entry -> entry.getName()) @@ -260,6 +279,8 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test { } catch (Exception e) { // Return an empty list if any error return Collections.EMPTY_LIST; + } finally { + getDevice().disableAdbRoot(); } } From bfbf9608015ba6ff740639625b9a3ea26db60b76 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Fri, 13 Nov 2020 19:08:31 +0000 Subject: [PATCH 2/2] Delete orphaned staging directories for staged session on reboot Bug: 173132101 Test: StagedInstallInternalTest#testOrphanedStagingDirectoryGetsCleanedUpOnReboot Change-Id: If840f35245c2d049401d0d2f6539fe8c4625151e --- .../android/server/pm/PackageInstallerService.java | 4 ++++ .../host/StagedInstallInternalTest.java | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index c282b99293495..b08a887909b9f 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -322,6 +322,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements final ArraySet unclaimedStages = newArraySet( stagingDir.listFiles(sStageFilter)); + // We also need to clean up orphaned staging directory for staged sessions + final File stagedSessionStagingDir = Environment.getDataStagingDirectory(volumeUuid); + unclaimedStages.addAll(newArraySet(stagedSessionStagingDir.listFiles())); + // Ignore stages claimed by active sessions for (int i = 0; i < mSessions.size(); i++) { final PackageInstallerSession session = mSessions.valueAt(i); diff --git a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java index 1faa4691d13eb..27ccbc78cf96f 100644 --- a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java +++ b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java @@ -268,6 +268,19 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test { assertThat(after).isEqualTo(before); } + @Test + public void testOrphanedStagingDirectoryGetsCleanedUpOnReboot() throws Exception { + //create random directories in /data/app-staging folder + getDevice().enableAdbRoot(); + getDevice().executeShellCommand("mkdir /data/app-staging/session_123"); + getDevice().executeShellCommand("mkdir /data/app-staging/random_name"); + getDevice().disableAdbRoot(); + + assertThat(getStagingDirectories()).isNotEmpty(); + getDevice().reboot(); + assertThat(getStagingDirectories()).isEmpty(); + } + private List getStagingDirectories() throws DeviceNotAvailableException { String baseDir = "/data/app-staging"; try {