diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 92da005babfa0..703a3ba4ce399 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -157,6 +157,7 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; public class PackageInstallerSession extends IPackageInstallerSession.Stub { @@ -262,6 +263,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private final Object mLock = new Object(); + /** + * Used to detect and reject concurrent access to this session object to ensure mutation + * to multiple objects like {@link #addChildSessionId} are done atomically. + */ + private final AtomicBoolean mTransactionLock = new AtomicBoolean(false); + /** Timestamp of the last time this session changed state */ @GuardedBy("mLock") private long updatedMillis; @@ -3073,39 +3080,74 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + private void acquireTransactionLock() { + if (!mTransactionLock.compareAndSet(false, true)) { + throw new UnsupportedOperationException("Concurrent access not supported"); + } + } + + private void releaseTransactionLock() { + mTransactionLock.compareAndSet(true, false); + } + @Override public void addChildSessionId(int childSessionId) { final PackageInstallerSession childSession = mSessionProvider.getSession(childSessionId); - if (childSession == null || !childSession.canBeAddedAsChild(sessionId)) { + if (childSession == null) { throw new IllegalStateException("Unable to add child session " + childSessionId - + " as it does not exist or is in an invalid state."); + + " as it does not exist."); } - synchronized (mLock) { - assertCallerIsOwnerOrRootLocked(); - assertPreparedAndNotSealedLocked("addChildSessionId"); - final int indexOfSession = mChildSessionIds.indexOfKey(childSessionId); - if (indexOfSession >= 0) { - return; + try { + acquireTransactionLock(); + childSession.acquireTransactionLock(); + + if (!childSession.canBeAddedAsChild(sessionId)) { + throw new IllegalStateException("Unable to add child session " + childSessionId + + " as it is in an invalid state."); } - childSession.setParentSessionId(this.sessionId); - addChildSessionIdLocked(childSessionId); + synchronized (mLock) { + assertCallerIsOwnerOrRootLocked(); + assertPreparedAndNotSealedLocked("addChildSessionId"); + + final int indexOfSession = mChildSessionIds.indexOfKey(childSessionId); + if (indexOfSession >= 0) { + return; + } + childSession.setParentSessionId(this.sessionId); + addChildSessionIdLocked(childSessionId); + } + } finally { + releaseTransactionLock(); + childSession.releaseTransactionLock(); } } @Override public void removeChildSessionId(int sessionId) { final PackageInstallerSession session = mSessionProvider.getSession(sessionId); - synchronized (mLock) { - final int indexOfSession = mChildSessionIds.indexOfKey(sessionId); + try { + acquireTransactionLock(); if (session != null) { - session.setParentSessionId(SessionInfo.INVALID_ID); + session.acquireTransactionLock(); } - if (indexOfSession < 0) { - // not added in the first place; no-op - return; + + synchronized (mLock) { + final int indexOfSession = mChildSessionIds.indexOfKey(sessionId); + if (session != null) { + session.setParentSessionId(SessionInfo.INVALID_ID); + } + if (indexOfSession < 0) { + // not added in the first place; no-op + return; + } + mChildSessionIds.removeAt(indexOfSession); + } + } finally { + releaseTransactionLock(); + if (session != null) { + session.releaseTransactionLock(); } - mChildSessionIds.removeAt(indexOfSession); } }