Merge "Return randomly generated session ids from createSession()."
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
index 977d8a0..aae33d7 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
@@ -96,10 +96,12 @@
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.charset.StandardCharsets;
+import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
+import java.util.Random;
import java.util.Set;
/**
@@ -122,8 +124,15 @@
// Contains all ids that are currently in use.
@GuardedBy("mBlobsLock")
+ private final ArraySet<Long> mActiveBlobIds = new ArraySet<>();
+ // Contains all ids that are currently in use and those that were in use but got deleted in the
+ // current boot session.
+ @GuardedBy("mBlobsLock")
private final ArraySet<Long> mKnownBlobIds = new ArraySet<>();
+ // Random number generator for new session ids.
+ private final Random mRandom = new SecureRandom();
+
private final Context mContext;
private final Handler mHandler;
private final Injector mInjector;
@@ -181,7 +190,16 @@
@GuardedBy("mBlobsLock")
private long generateNextSessionIdLocked() {
- return ++mCurrentMaxSessionId;
+ // Logic borrowed from PackageInstallerService.
+ int n = 0;
+ long sessionId;
+ do {
+ sessionId = Math.abs(mRandom.nextLong());
+ if (mKnownBlobIds.indexOf(sessionId) < 0 && sessionId != 0) {
+ return sessionId;
+ }
+ } while (n++ < 32);
+ throw new IllegalStateException("Failed to allocate session ID");
}
private void registerReceivers() {
@@ -228,15 +246,22 @@
}
@VisibleForTesting
- void addKnownIdsForTest(long... knownIds) {
+ void addActiveIdsForTest(long... activeIds) {
synchronized (mBlobsLock) {
- for (long id : knownIds) {
- mKnownBlobIds.add(id);
+ for (long id : activeIds) {
+ addActiveBlobIdLocked(id);
}
}
}
@VisibleForTesting
+ Set<Long> getActiveIdsForTest() {
+ synchronized (mBlobsLock) {
+ return mActiveBlobIds;
+ }
+ }
+
+ @VisibleForTesting
Set<Long> getKnownIdsForTest() {
synchronized (mBlobsLock) {
return mKnownBlobIds;
@@ -246,7 +271,7 @@
@GuardedBy("mBlobsLock")
private void addSessionForUserLocked(BlobStoreSession session, int userId) {
getUserSessionsLocked(userId).put(session.getSessionId(), session);
- mKnownBlobIds.add(session.getSessionId());
+ addActiveBlobIdLocked(session.getSessionId());
}
@GuardedBy("mBlobsLock")
@@ -258,7 +283,13 @@
private void addBlobForUserLocked(BlobMetadata blobMetadata,
ArrayMap<BlobHandle, BlobMetadata> userBlobs) {
userBlobs.put(blobMetadata.getBlobHandle(), blobMetadata);
- mKnownBlobIds.add(blobMetadata.getBlobId());
+ addActiveBlobIdLocked(blobMetadata.getBlobId());
+ }
+
+ @GuardedBy("mBlobsLock")
+ private void addActiveBlobIdLocked(long id) {
+ mActiveBlobIds.add(id);
+ mKnownBlobIds.add(id);
}
private long createSessionInternal(BlobHandle blobHandle,
@@ -392,7 +423,7 @@
synchronized (mBlobsLock) {
getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid()))
.remove(session.getSessionId());
- mKnownBlobIds.remove(session.getSessionId());
+ mActiveBlobIds.remove(session.getSessionId());
if (LOGV) {
Slog.v(TAG, "Session is invalid; deleted " + session);
}
@@ -710,7 +741,7 @@
if (session.getOwnerUid() == uid
&& session.getOwnerPackageName().equals(packageName)) {
session.getSessionFile().delete();
- mKnownBlobIds.remove(session.getSessionId());
+ mActiveBlobIds.remove(session.getSessionId());
indicesToRemove.add(i);
}
}
@@ -730,7 +761,7 @@
// Delete the blob if it doesn't have any active leases.
if (!blobMetadata.hasLeases()) {
blobMetadata.getBlobFile().delete();
- mKnownBlobIds.remove(blobMetadata.getBlobId());
+ mActiveBlobIds.remove(blobMetadata.getBlobId());
indicesToRemove.add(i);
}
}
@@ -753,7 +784,7 @@
for (int i = 0, count = userSessions.size(); i < count; ++i) {
final BlobStoreSession session = userSessions.valueAt(i);
session.getSessionFile().delete();
- mKnownBlobIds.remove(session.getSessionId());
+ mActiveBlobIds.remove(session.getSessionId());
}
}
@@ -763,7 +794,7 @@
for (int i = 0, count = userBlobs.size(); i < count; ++i) {
final BlobMetadata blobMetadata = userBlobs.valueAt(i);
blobMetadata.getBlobFile().delete();
- mKnownBlobIds.remove(blobMetadata.getBlobId());
+ mActiveBlobIds.remove(blobMetadata.getBlobId());
}
}
if (LOGV) {
@@ -783,7 +814,7 @@
for (File file : blobsDir.listFiles()) {
try {
final long id = Long.parseLong(file.getName());
- if (mKnownBlobIds.indexOf(id) < 0) {
+ if (mActiveBlobIds.indexOf(id) < 0) {
filesToDelete.add(file);
deletedBlobIds.add(id);
}
@@ -818,7 +849,7 @@
if (shouldRemove) {
blobMetadata.getBlobFile().delete();
- mKnownBlobIds.remove(blobMetadata.getBlobId());
+ mActiveBlobIds.remove(blobMetadata.getBlobId());
deletedBlobIds.add(blobMetadata.getBlobId());
}
return shouldRemove;
@@ -848,7 +879,7 @@
if (shouldRemove) {
blobStoreSession.getSessionFile().delete();
- mKnownBlobIds.remove(blobStoreSession.getSessionId());
+ mActiveBlobIds.remove(blobStoreSession.getSessionId());
indicesToRemove.add(j);
deletedBlobIds.add(blobStoreSession.getSessionId());
}
@@ -895,7 +926,7 @@
}
blobMetadata.getBlobFile().delete();
userBlobs.remove(blobHandle);
- mKnownBlobIds.remove(blobMetadata.getBlobId());
+ mActiveBlobIds.remove(blobMetadata.getBlobId());
writeBlobsInfoAsync();
}
}
diff --git a/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java
index 3778e17..e5450a9 100644
--- a/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java
@@ -149,7 +149,7 @@
final BlobMetadata blobMetadata2 = createBlobMetadataMock(blobId2, blobFile2, false);
mUserBlobs.put(blobHandle2, blobMetadata2);
- mService.addKnownIdsForTest(sessionId1, sessionId2, sessionId3, sessionId4,
+ mService.addActiveIdsForTest(sessionId1, sessionId2, sessionId3, sessionId4,
blobId1, blobId2);
// Invoke test method
@@ -180,8 +180,10 @@
assertThat(mUserBlobs.get(blobHandle1)).isNotNull();
assertThat(mUserBlobs.get(blobHandle2)).isNull();
- assertThat(mService.getKnownIdsForTest()).containsExactly(
+ assertThat(mService.getActiveIdsForTest()).containsExactly(
sessionId2, sessionId3, blobId1);
+ assertThat(mService.getKnownIdsForTest()).containsExactly(
+ sessionId1, sessionId2, sessionId3, sessionId4, blobId1, blobId2);
}
@Test
@@ -198,12 +200,12 @@
doReturn(String.valueOf(testId3)).when(file3).getName();
doReturn(new File[] {file1, file2, file3}).when(mBlobsDir).listFiles();
- mService.addKnownIdsForTest(testId1, testId3);
+ mService.addActiveIdsForTest(testId1, testId3);
// Invoke test method
mService.handleIdleMaintenanceLocked();
- // Verify unknown blobs are delete
+ // Verify unknown blobs are deleted
verify(file1, never()).delete();
verify(file2).delete();
verify(file3, never()).delete();
@@ -242,7 +244,7 @@
sessionId3, sessionFile3, blobHandle3);
mUserSessions.append(sessionId3, session3);
- mService.addKnownIdsForTest(sessionId1, sessionId2, sessionId3);
+ mService.addActiveIdsForTest(sessionId1, sessionId2, sessionId3);
// Invoke test method
mService.handleIdleMaintenanceLocked();
@@ -255,7 +257,9 @@
assertThat(mUserSessions.size()).isEqualTo(1);
assertThat(mUserSessions.get(sessionId2)).isNotNull();
- assertThat(mService.getKnownIdsForTest()).containsExactly(sessionId2);
+ assertThat(mService.getActiveIdsForTest()).containsExactly(sessionId2);
+ assertThat(mService.getKnownIdsForTest()).containsExactly(
+ sessionId1, sessionId2, sessionId3);
}
@Test
@@ -282,7 +286,7 @@
final BlobMetadata blobMetadata3 = createBlobMetadataMock(blobId3, blobFile3, false);
mUserBlobs.put(blobHandle3, blobMetadata3);
- mService.addKnownIdsForTest(blobId1, blobId2, blobId3);
+ mService.addActiveIdsForTest(blobId1, blobId2, blobId3);
// Invoke test method
mService.handleIdleMaintenanceLocked();
@@ -295,7 +299,8 @@
assertThat(mUserBlobs.size()).isEqualTo(1);
assertThat(mUserBlobs.get(blobHandle2)).isNotNull();
- assertThat(mService.getKnownIdsForTest()).containsExactly(blobId2);
+ assertThat(mService.getActiveIdsForTest()).containsExactly(blobId2);
+ assertThat(mService.getKnownIdsForTest()).containsExactly(blobId1, blobId2, blobId3);
}
private BlobStoreSession createBlobStoreSessionMock(String ownerPackageName, int ownerUid,