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

Commit cae4e463 authored by Jooyung Han's avatar Jooyung Han
Browse files

Make sure session.abandon() will abort apex session 

In this change, two races are handled to make sure that apex session is
aborted correctly.

1. session.abandon() is called before marking it as ready.

PackageSessionVerifier will mark the session as "ready" after the
verification. Abandon() marks the session as "destroyed", marking it as
ready can fail, which results in failing to call either of
markStagedSessionReady() or abortStagedSession() of IApexService.

PackageSessionVerifier now throws an exception when it fails to mark the
session as ready so that the exception handler can ensure the active
session is aborted (calling abortStagedSession()).

2. session.abandon() is called just before dispatchPendingAbandonCallback()

The session (and its apex session) is marked "ready". However, the
sesion is not yet added to StagingManager. Hence, even though abandon()
calls StagingManager.abortCommitedSession(), StagingManager does
nothing because it doesn't know the session. (Not aborted)

In this change, StagingManager makes sure that the apex session is
aborted.

Note that these two races don't cause any severe problem now, but leave the
two session data (PM sessions and APEX sessions) out of sync. This
prevented adding check like ag/34097710.

Bug: 425478146
Flag: EXEMPT BUGFIX
Test: StagedInstallInternalTest, StagingManagerTest
Change-Id: Id41c6e493e3f5e4de78b3e9d240e6222182238eb
parent 07ebbee2

services/core/java/com/android/server/pm/PackageSessionVerifier.java

+7 −0
Original line number Diff line number Diff line
@@ -363,11 +363,18 @@ final class PackageSessionVerifier { 
        // only apex part of the train will be applied, leaving device in an inconsistent state.

        Slog.d(TAG, "Marking session " + session.sessionId() + " as ready");

        session.setSessionReady();

        // If the session is abandoned while it's being staged/verified, the session can't be marked

        // as ready. Let's abort the session by throwing PackageManagerException. The apex session

        // will be aborted as well in the exception handler.

        if (session.isSessionReady()) {

            final boolean hasApex = session.containsApexSession();

            if (hasApex) {

                mApexManager.markStagedSessionReady(session.sessionId());

            }

        } else {

            Slog.e(TAG, "Failed to mark session " + session.sessionId() + " as ready.");

            throw new PackageManagerException(PackageManager.INSTALL_FAILED_INTERNAL_ERROR,

                    "Session is abandoned.");

        }

    }

services/core/java/com/android/server/pm/StagingManager.java

+19 −12
Original line number Diff line number Diff line
@@ -532,22 +532,29 @@ public class StagingManager { 
            throw new IllegalStateException("Committed session must be destroyed before aborting it"

                    + " from StagingManager");

        }

        if (getStagedSession(sessionId) == null) {

            Slog.w(TAG, "Session " + sessionId + " has been abandoned already");

            return;

        }



        // A session could be marked ready once its pre-reboot verification ends

        if (session.isSessionReady()) {

        // Abort the apex session. The apex session might be in one of three states:

        // - yet to be created (abandoned before submitStagedSession() is not called)

        // - created(VERIFIED) but not marked ready (abandoned before markStagedSessionReady())

        // - ready(STAGED), but not committed yet (abandoned after markStagedSessionReady())

        // Even when it's ready, the StagedSession may not be committed yet when abandon() is called

        // before PackageSessionVerifier.Callback is called.

        if (!ensureActiveApexSessionIsAborted(session)) {

            // Failed to ensure apex session is aborted, so it can still be staged. We can still

            // safely cleanup the staged session since pre-reboot verification is complete.

            // Also, cleaning up the stageDir prevents the apex from being activated.

            Slog.e(TAG, "Failed to abort apex session " + session.sessionId());

        }

            if (session.containsApexSession()) {

                notifyStagedApexObservers();



        if (getStagedSession(sessionId) == null) {

            Slog.w(TAG, "Session " + sessionId + " has been abandoned already,"

                    + " or not committed yet");

            return;

        }



        // Since we have notified observers on commitSession(), notify them on abort as well.

        if (session.isSessionReady() && session.containsApexSession()) {

            notifyStagedApexObservers();

        }



        // Session was successfully aborted from apexd (if required) and pre-reboot verification

services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java

+19 −0
Original line number Diff line number Diff line
@@ -721,6 +721,25 @@ public class StagingManagerTest { 
        verify(observer, never()).onApexStaged(any());

    }



    @Test

    public void abortCommittedSession_ensuresApexSessionIsAborted_whenItsNotReady()

            throws Exception {

        // Create a session and abandon it. It's not ready and committed to StagingManager yet.

        FakeStagedSession session = new FakeStagedSession(239);

        session.setIsApex(true);

        session.setDestroyed(true);

        assertThat(session.isSessionReady()).isFalse();



        ApexSessionInfo stagedSessionInfo = new ApexSessionInfo();

        stagedSessionInfo.sessionId = 239;

        stagedSessionInfo.isVerified = true;

        when(mApexManager.getStagedSessionInfo(239)).thenReturn(stagedSessionInfo);



        mStagingManager.abortCommittedSession(session);



        verify(mApexManager, times(1)).abortStagedSession(239);

    }



    private StagingManager.StagedSession createSession(int sessionId, String packageName,

            long committedMillis) {

        PackageInstaller.SessionParams params = new PackageInstaller.SessionParams(

tests/StagedInstallTest/app/src/com/android/tests/stagedinstallinternal/StagedInstallInternalTest.java

+42 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.timeout; 
import static org.mockito.Mockito.verify;



import android.Manifest;

import android.content.Intent;

import android.content.pm.ApexStagedEvent;

import android.content.pm.ApplicationInfo;

import android.content.pm.IPackageManagerNative;
@@ -43,6 +44,7 @@ import androidx.test.platform.app.InstrumentationRegistry; 


import com.android.cts.install.lib.Install;

import com.android.cts.install.lib.InstallUtils;

import com.android.cts.install.lib.LocalIntentSender;

import com.android.cts.install.lib.TestApp;

import com.android.cts.install.lib.Uninstall;
@@ -61,6 +63,7 @@ import java.io.FileReader; 
import java.io.FileWriter;

import java.nio.file.Files;

import java.util.List;

import java.util.concurrent.TimeUnit;

import java.util.function.Consumer;



@RunWith(JUnit4.class)
@@ -217,6 +220,45 @@ public class StagedInstallInternalTest { 
        InstallUtils.getPackageInstaller().abandonSession(id4);

    }



    @Test

    public void testAbandonShouldCleanUpApexSession_AbandonDuringVerification() throws Exception {

        assertThat(InstallUtils.getInstalledVersion(SHIM_APEX_PACKAGE_NAME)).isEqualTo(1);

        int sessionId = Install.single(APEX_V2).setStaged().createSession();

        try (PackageInstaller.Session session =

                     InstallUtils.openPackageInstallerSession(sessionId)) {

            LocalIntentSender sender = new LocalIntentSender();

            session.commit(sender.getIntentSender());

            // wait 500 ms to give apexd a chance to start the verification

            Thread.sleep(500);

            session.abandon();

            Intent result = sender.pollResult(1, TimeUnit.MINUTES);

            assertThat(result).isNotNull();

            InstallUtils.assertStatusFailure(result);

        }

    }



    @Test

    public void testAbandonShouldCleanUpApexSession_AbandonAfterVerification() throws Exception {

        assertThat(InstallUtils.getInstalledVersion(SHIM_APEX_PACKAGE_NAME)).isEqualTo(1);

        int sessionId = Install.single(APEX_V2).setStaged().createSession();

        try (PackageInstaller.Session session =

                     InstallUtils.openPackageInstallerSession(sessionId)) {

            LocalIntentSender sender = new LocalIntentSender();

            session.commit(sender.getIntentSender());

            // commit() should succeed first

            Intent result = sender.pollResult(1, TimeUnit.MINUTES);

            assertThat(result).isNotNull();

            InstallUtils.assertStatusSuccess(result);



            session.abandon();

            // abandon() should report failure

            result = sender.pollResult(1, TimeUnit.MINUTES);

            assertThat(result).isNotNull();

            InstallUtils.assertStatusFailure(result);

        }

    }





    @Test

    public void testStagedSessionShouldCleanUpOnVerificationFailure() throws Exception {

        // APEX verification

tests/StagedInstallTest/src/com/android/tests/stagedinstallinternal/host/StagedInstallInternalTest.java

+21 −0
Original line number Diff line number Diff line
@@ -350,6 +350,27 @@ public class StagedInstallInternalTest extends BaseHostJUnit4Test { 
        assertThat(after).isEqualTo(before);

    }



    @Test

    public void testAbandonShouldCleanUpApexSession_AbandonDuringVerification() throws Exception {

        String before = getDevice().executeShellCommand("apexd --dump sessions");

        getDevice().setProperty("apexd.test_hook.submit_staged_session", "sleep_ms 1000");

        try {

            runPhase("testAbandonShouldCleanUpApexSession_AbandonDuringVerification");

            String after = getDevice().executeShellCommand("apexd --dump sessions");

            assertThat(after).isEqualTo(before);

        } finally {

            getDevice().setProperty("apexd.test_hook.submit_staged_session", "");

        }

    }



    @Test

    public void testAbandonShouldCleanUpApexSession_AbandonAfterVerification() throws Exception {

        String before = getDevice().executeShellCommand("apexd --dump sessions");

        runPhase("testAbandonShouldCleanUpApexSession_AbandonAfterVerification");

        String after = getDevice().executeShellCommand("apexd --dump sessions");

        assertThat(after).isEqualTo(before);

    }



    @Test

    public void testStagedSessionShouldCleanUpOnVerificationFailure() throws Exception {

        assumeTrue("Device does not support updating APEX",