From 9dedc3fc2a74afa9eeebee5c56057312283e2871 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Fri, 13 Nov 2020 17:32:05 +0000 Subject: Clean up staged session data on validation failure Bug: 173132101 Test: atest StagedInstallInternalTest#testStagedInstallationShouldCleanUpOnValidationFailure Test: atest StagedInstallInternalTest#testStagedInstallationShouldCleanUpOnValidationFailureMultiPackage Change-Id: Idd6597cd0d2dda34a8a7626b585401eeee39c31f Merged-In: Idd6597cd0d2dda34a8a7626b585401eeee39c31f (cherry picked from commit b6dde00d038b899e5235b6a12d2ec6a2864f2022) (cherry picked from commit f54412183a19916f6ce704f113967b8b19ef6ed8) --- .../android/server/pm/PackageInstallerSession.java | 4 +++ .../StagedInstallInternalTest.java | 13 ++++++++ .../host/StagedInstallInternalTest.java | 38 ++++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index ea53132ae409..c9f09545c3e6 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -1581,6 +1581,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 onStorageUnhealthy() { 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 02597d548361..e67354982b05 100644 --- a/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java +++ b/tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java @@ -96,6 +96,19 @@ public class StagedInstallInternalTest { assertSessionReady(sessionId); } + @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 9b432f7d0ca5..9ae4c2fc80ec 100644 --- a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java +++ b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java @@ -21,6 +21,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertTrue; import com.android.ddmlib.Log; +import com.android.tradefed.device.DeviceNotAvailableException; import com.android.tradefed.testtype.DeviceJUnit4ClassRunner; import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test; import com.android.tradefed.util.ProcessInfo; @@ -30,6 +31,10 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + @RunWith(DeviceJUnit4ClassRunner.class) public class StagedInstallInternalTest extends BaseHostJUnit4Test { @@ -87,6 +92,39 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test { runPhase("testSystemServerRestartDoesNotAffectStagedSessions_Verify"); } + @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()) + .collect(Collectors.toList()); + } catch (Exception e) { + // Return an empty list if any error + return Collections.EMPTY_LIST; + } finally { + getDevice().disableAdbRoot(); + } + } + private void restartSystemServer() throws Exception { // Restart the system server long oldStartTime = getDevice().getProcessByName("system_server").getStartTime(); -- cgit v1.2.3 From 69c24da80c37260459e7b09e5b144a02124e25d9 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Fri, 13 Nov 2020 19:08:31 +0000 Subject: Delete orphaned staging directories for staged session on reboot Bug: 173132101 Test: StagedInstallInternalTest#testOrphanedStagingDirectoryGetsCleanedUpOnReboot Change-Id: If840f35245c2d049401d0d2f6539fe8c4625151e Merged-In: If840f35245c2d049401d0d2f6539fe8c4625151e (cherry picked from commit bfbf9608015ba6ff740639625b9a3ea26db60b76) (cherry picked from commit 37daf1ba806e4b2f952a7cc4e1e2babfbea8d8b1) --- .../java/com/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 330f99523507..9f0efa5fad83 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -299,6 +299,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 9ae4c2fc80ec..28a5424199a4 100644 --- a/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java +++ b/tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java @@ -109,6 +109,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 { -- cgit v1.2.3