Merge changes I726dd151,I0ea1a663

* changes:
  Add session cleanup tests for multi-package sessions (2/n)
  A destroyed session shouldn't fail other sessions
This commit is contained in:
JW Wang
2021-11-18 02:27:14 +00:00
committed by Android (Google) Code Review
5 changed files with 74 additions and 18 deletions

View File

@@ -558,10 +558,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
Slog.d(TAG, "Marking session " + sessionId + " as failed: " + errorMessage);
childSessions = getChildSessionsLocked();
}
destroyInternal();
for (PackageInstallerSession child : childSessions) {
child.destroyInternal();
}
destroy();
mCallback.onStagedSessionChanged(PackageInstallerSession.this);
}
@@ -579,10 +576,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
Slog.d(TAG, "Marking session " + sessionId + " as applied");
childSessions = getChildSessionsLocked();
}
destroyInternal();
for (PackageInstallerSession child : childSessions) {
child.destroyInternal();
}
destroy();
mCallback.onStagedSessionChanged(PackageInstallerSession.this);
}
@@ -2111,18 +2105,14 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
private void onSessionVerificationFailure(int error, String msg) {
final String msgWithErrorCode = PackageManager.installStatusToString(error, msg);
Slog.e(TAG, "Failed to verify session " + sessionId + " [" + msgWithErrorCode + "]");
// Session is sealed and committed but could not be verified, we need to destroy it.
destroyInternal();
if (isMultiPackage()) {
for (PackageInstallerSession childSession : getChildSessions()) {
childSession.destroyInternal();
}
}
if (isStaged()) {
// This will clean up the session when it reaches the terminal state
mStagedSession.setSessionFailed(
SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, msgWithErrorCode);
mStagedSession.notifyEndPreRebootVerification();
} else {
// Session is sealed and committed but could not be verified, we need to destroy it.
destroy();
// Dispatch message to remove session from PackageInstallerService.
dispatchSessionFinished(error, msg, null);
}
@@ -4265,6 +4255,24 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
return params.isStaged ? mStagedSession.getSessionErrorMessage() : "";
}
/**
* Free up storage used by this session and its children.
* Must not be called on a child session.
*/
private void destroy() {
// TODO(b/173194203): destroyInternal() should be used by destroy() only.
// For the sake of consistency, a session should be destroyed as a whole. The caller
// should always call destroy() for cleanup without knowing it has child sessions or not.
assertNotChild("destroy");
destroyInternal();
for (PackageInstallerSession child : getChildSessions()) {
child.destroyInternal();
}
}
/**
* Free up storage used by this session.
*/
private void destroyInternal() {
final IncrementalFileStorages incrementalFileStorages;
synchronized (mLock) {

View File

@@ -527,6 +527,9 @@ final class PackageSessionVerifier {
@VisibleForTesting
void checkRollbacks(StagingManager.StagedSession session)
throws PackageManagerException {
if (session.isDestroyed() || session.isInTerminalState()) {
return;
}
for (StagingManager.StagedSession stagedSession : mStagedSessions) {
if (stagedSession.isDestroyed() || stagedSession.isInTerminalState()) {
continue;
@@ -565,6 +568,9 @@ final class PackageSessionVerifier {
@VisibleForTesting
void checkOverlaps(StagingManager.StagedSession parent,
StagingManager.StagedSession child) throws PackageManagerException {
if (parent.isDestroyed() || parent.isInTerminalState()) {
return;
}
final String packageName = child.getPackageName();
if (packageName == null) {
throw new PackageManagerException(

View File

@@ -21,6 +21,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -77,18 +78,26 @@ public class PackageSessionVerifierTest {
public void checkRollbacks() throws Exception {
StagingManager.StagedSession session1 = createStagedSession(111, "com.foo", 1);
StagingManager.StagedSession session2 = createStagedSession(222, "com.bar", 2);
StagingManager.StagedSession session3 = createStagedSession(333, "com.baz", 3);
session2.sessionParams().setInstallReason(PackageManager.INSTALL_REASON_ROLLBACK);
session3.sessionParams().setInstallReason(PackageManager.INSTALL_REASON_ROLLBACK);
when(session2.isDestroyed()).thenReturn(true);
mSessionVerifier.storeSession(session1);
mSessionVerifier.storeSession(session2);
mSessionVerifier.storeSession(session3);
// Non-rollback session shouldn't be failed by a destroyed session
mSessionVerifier.checkRollbacks(session2);
verify(session1, never()).setSessionFailed(anyInt(), anyString());
// Non-rollback session should fail
mSessionVerifier.checkRollbacks(session2);
mSessionVerifier.checkRollbacks(session3);
verify(session1, times(1)).setSessionFailed(anyInt(), anyString());
// Yet another non-rollback session should fail
StagingManager.StagedSession session3 = createStagedSession(333, "com.baz", 3);
StagingManager.StagedSession session4 = createStagedSession(444, "com.fur", 4);
assertThrows(PackageManagerException.class,
() -> mSessionVerifier.checkRollbacks(session3));
() -> mSessionVerifier.checkRollbacks(session4));
}
@Test
@@ -105,6 +114,11 @@ public class PackageSessionVerifierTest {
StagingManager.StagedSession session3 = createStagedSession(333, "com.foo", 3);
assertThrows(PackageManagerException.class,
() -> mSessionVerifier.checkOverlaps(session3, session3));
// session4 is earlier than session1, but it shouldn't fail session1
StagingManager.StagedSession session4 = createStagedSession(444, "com.foo", 0);
when(session4.isDestroyed()).thenReturn(true);
mSessionVerifier.checkOverlaps(session4, session4);
verify(session1, never()).setSessionFailed(anyInt(), anyString());
}
private PackageInstallerSession createSession(boolean isStaged, boolean isApex,

View File

@@ -157,8 +157,18 @@ public class StagedInstallInternalTest {
@Test
public void testStagedSessionShouldCleanUpOnVerificationFailure() throws Exception {
// APEX verification
InstallUtils.commitExpectingFailure(AssertionError.class, "apexd verification failed",
Install.single(APEX_WRONG_SHA_V2).setStaged());
InstallUtils.commitExpectingFailure(AssertionError.class, "apexd verification failed",
Install.multi(APEX_WRONG_SHA_V2, TestApp.A1).setStaged());
// APK verification
Install.single(TestApp.A2).commit();
assertThat(InstallUtils.getInstalledVersion(TestApp.A)).isEqualTo(2);
InstallUtils.commitExpectingFailure(AssertionError.class, "Downgrade detected",
Install.single(TestApp.A1).setStaged());
InstallUtils.commitExpectingFailure(AssertionError.class, "Downgrade detected",
Install.multi(TestApp.A1, TestApp.B1).setStaged());
}
@Test
@@ -175,6 +185,12 @@ public class StagedInstallInternalTest {
assertThat(info.isStagedSessionApplied()).isTrue();
}
@Test
public void testStagedSessionShouldCleanUpOnOnSuccessMultiPackage_Commit() throws Exception {
int sessionId = Install.multi(TestApp.A1, TestApp.Apex2).setStaged().commit();
storeSessionId(sessionId);
}
@Test
public void testStagedInstallationShouldCleanUpOnValidationFailure() throws Exception {
InstallUtils.commitExpectingFailure(AssertionError.class, "INSTALL_FAILED_INVALID_APK",

View File

@@ -300,6 +300,18 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test {
assertThat(after).isEqualTo(before);
}
@Test
@LargeTest
public void testStagedSessionShouldCleanUpOnOnSuccessMultiPackage() throws Exception {
List<String> before = getStagingDirectories();
runPhase("testStagedSessionShouldCleanUpOnOnSuccessMultiPackage_Commit");
assertThat(getStagingDirectories()).isNotEqualTo(before);
getDevice().reboot();
runPhase("testStagedSessionShouldCleanUpOnOnSuccess_Verify");
List<String> after = getStagingDirectories();
assertThat(after).isEqualTo(before);
}
@Test
public void testStagedInstallationShouldCleanUpOnValidationFailure() throws Exception {
List<String> before = getStagingDirectories();