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

Commit 369d0425 authored by Samiul Islam's avatar Samiul Islam
Browse files

Convert DependencyInstallerCallback to two-way

We would like to throw IllegalArgumentException for invalid session-ids.
Can't do that with one-way binder calls. Once we validate the
session-ids, we off-load the work on the handler thread.

Bug: 372862145
Test: atest PackageManagerShellCommandInstallTest
FLAG: android.content.pm.sdk_dependency_installer
Change-Id: Idac7ecbaee026cf37a876179b312e8d5178d86b7
parent efd5e043
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -53,7 +53,14 @@ public final class DependencyInstallerCallback implements Parcelable {
     * Callback to indicate that all the requested dependencies have been resolved and their
     * sessions created. See {@link  DependencyInstallerService#onDependenciesRequired}.
     *
     * The system will wait for the sessions to be installed before resuming the original session
     * which requested dependency installation.
     *
     * If any of the session fails to install, the system may fail the original session. The caller
     * is expected to handle clean up of any other pending sessions remanining.
     *
     * @param sessionIds the install session IDs for all requested dependencies
     * @throws IllegalArgumentException if session id doesn't exist or has already failed.
     */
    public void onAllDependenciesResolved(@NonNull int[] sessionIds) {
        try {
+2 −2
Original line number Diff line number Diff line
@@ -24,7 +24,7 @@ import java.util.List;
*
* {@hide}
*/
oneway interface IDependencyInstallerCallback {
interface IDependencyInstallerCallback {
    /**
     * Callback to indicate that all the requested dependencies have been resolved and have been
     * committed for installation. See {@link  DependencyInstallerService#onDependenciesRequired}.
+94 −43
Original line number Diff line number Diff line
@@ -110,7 +110,7 @@ public class InstallDependencyHelper {
        }

        IDependencyInstallerCallback serviceCallback =
                new DependencyInstallerCallbackCallOnce(handler, callback);
                new DependencyInstallerCallbackCallOnce(handler, callback, userId);
        boolean scheduleSuccess;
        synchronized (mRemoteServiceLock) {
            scheduleSuccess = mRemoteService.run(service -> {
@@ -292,79 +292,130 @@ public class InstallDependencyHelper {

        private final Handler mHandler;
        private final CallOnceProxy mCallback;
        private final int mUserId;

        @GuardedBy("this")
        private boolean mCalled = false;
        private boolean mDependencyInstallerCallbackInvoked = false;

        DependencyInstallerCallbackCallOnce(Handler handler, CallOnceProxy callback) {
        DependencyInstallerCallbackCallOnce(Handler handler, CallOnceProxy callback, int userId) {
            mHandler = handler;
            mCallback = callback;
            mUserId = userId;
        }

        // TODO(b/372862145): Consider turning the binder call to two-way so that we can
        //  throw IllegalArgumentException
        @Override
        public void onAllDependenciesResolved(int[] sessionIds) throws RemoteException {
            synchronized (this) {
                if (mCalled) {
                    return;
                if (mDependencyInstallerCallbackInvoked) {
                    throw new IllegalStateException(
                            "Callback is being or has been already processed");
                }
                mCalled = true;
                mDependencyInstallerCallbackInvoked = true;
            }

            ArraySet<Integer> set = new ArraySet<>();
            for (int i = 0; i < sessionIds.length; i++) {

            if (DEBUG) {
                    Slog.i(TAG, "onAllDependenciesResolved called with " + sessionIds[i]);
                Slog.d(TAG, "onAllDependenciesResolved started");
            }
                set.add(sessionIds[i]);

            // Before creating any tracker, validate the arguments
            ArraySet<Integer> validSessionIds = validateSessionIds(sessionIds);

            if (validSessionIds.isEmpty()) {
                mCallback.onResult(null);
                return;
            }

            DependencyInstallTracker tracker = new DependencyInstallTracker(mCallback, set);
            // Create a tracker now if there are any pending sessions remaining.
            DependencyInstallTracker tracker = new DependencyInstallTracker(
                    mCallback, validSessionIds);
            synchronized (mTrackers) {
                mTrackers.add(tracker);
            }

            // In case any of the session ids have already been installed, check if they
            // are valid.
            mHandler.post(() -> {
                if (DEBUG) {
                    Slog.i(TAG, "onAllDependenciesResolved cleaning up invalid sessions");
            // By the time the tracker was created, some of the sessions in validSessionIds
            // could have finished. Avoid waiting for them indefinitely.
            for (int sessionId : validSessionIds) {
                SessionInfo sessionInfo = mPackageInstallerService.getSessionInfo(sessionId);

                // Don't wait for sessions that finished already
                if (sessionInfo == null) {
                    notifySessionComplete(sessionId, /*success=*/ true);
                }
            }
        }

        @Override
        public void onFailureToResolveAllDependencies() throws RemoteException {
            synchronized (this) {
                if (mDependencyInstallerCallbackInvoked) {
                    throw new IllegalStateException(
                            "Callback is being or has been already processed");
                }
                mDependencyInstallerCallbackInvoked = true;
            }
            onError(mCallback, "Failed to resolve all dependencies automatically");
        }

        private ArraySet<Integer> validateSessionIds(int[] sessionIds) {
            // Before creating any tracker, validate the arguments
            ArraySet<Integer> validSessionIds = new ArraySet<>();

            List<SessionInfo> historicalSessions = null;
            for (int i = 0; i < sessionIds.length; i++) {
                int sessionId = sessionIds[i];
                SessionInfo sessionInfo = mPackageInstallerService.getSessionInfo(sessionId);

                // Continue waiting if session exists and hasn't passed or failed yet.
                    if (sessionInfo != null && !sessionInfo.isSessionApplied
                            && !sessionInfo.isSessionFailed) {
                if (sessionInfo != null) {
                    if (sessionInfo.isSessionFailed) {
                        throwValidationError("Session already finished: " + sessionId);
                    }

                    // Wait for session to finish install if it's not already successful.
                    if (!sessionInfo.isSessionApplied) {
                        if (DEBUG) {
                            Slog.d(TAG, "onAllDependenciesResolved pending session: " + sessionId);
                        }
                        validSessionIds.add(sessionId);
                    }

                    // An applied session found. No need to check historical session anymore.
                    continue;
                }

                if (DEBUG) {
                        Slog.i(TAG, "onAllDependenciesResolved cleaning up finished"
                    Slog.d(TAG, "onAllDependenciesResolved cleaning up finished"
                            + " session: " + sessionId);
                }

                    // If session info is null, we assume it to be success.
                    // TODO(b/372862145): Check historical sessions to be more precise.
                    boolean success = sessionInfo == null || sessionInfo.isSessionApplied;
                if (historicalSessions == null) {
                    historicalSessions = mPackageInstallerService.getHistoricalSessions(
                            mUserId).getList();
                }

                sessionInfo = historicalSessions.stream().filter(
                        s -> s.sessionId == sessionId).findFirst().orElse(null);

                    notifySessionComplete(sessionId, /*success=*/success);
                if (sessionInfo == null) {
                    throwValidationError("Failed to find session: " + sessionId);
                }

                // Historical session must have been successful, otherwise throw IAE.
                if (!sessionInfo.isSessionApplied) {
                    throwValidationError("Session already finished: " + sessionId);
                }
            });
            }

        @Override
        public void onFailureToResolveAllDependencies() throws RemoteException {
            synchronized (this) {
                if (mCalled) {
                    return;
            return validSessionIds;
        }
                onError(mCallback, "Failed to resolve all dependencies automatically");
                mCalled = true;

        private void throwValidationError(String msg) {
            // Allow client to invoke callback again.
            synchronized (this) {
                mDependencyInstallerCallbackInvoked = false;
            }
            throw new IllegalArgumentException(msg);
        }
    }

@@ -377,6 +428,7 @@ public class InstallDependencyHelper {
    // TODO(b/372862145): Determine and add support for rebooting while dependency is being resolved
    private static class DependencyInstallTracker {
        private final CallOnceProxy mCallback;
        @GuardedBy("this")
        private final ArraySet<Integer> mPendingSessionIds;

        DependencyInstallTracker(CallOnceProxy callback, ArraySet<Integer> pendingSessionIds) {
@@ -411,6 +463,5 @@ public class InstallDependencyHelper {
                return true; // Keep on tracking
            }
        }

    }
}