Merge "Return randomly generated session ids from createSession()."

1 files changed, 46 insertions, 15 deletions

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 977d8a074a83..aae33d7b0a89 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.FileOutputStream;
 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 @@ public class BlobStoreManagerService extends SystemService {
 
     // 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 @@ public class BlobStoreManagerService extends SystemService {
 
     @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 @@ public class BlobStoreManagerService extends SystemService {
     }
 
     @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 @@ public class BlobStoreManagerService extends SystemService {
     @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 @@ public class BlobStoreManagerService extends SystemService {
     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 @@ public class BlobStoreManagerService extends SystemService {
                 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 @@ public class BlobStoreManagerService extends SystemService {
                 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 @@ public class BlobStoreManagerService extends SystemService {
                 // 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 @@ public class BlobStoreManagerService extends SystemService {
                 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 @@ public class BlobStoreManagerService extends SystemService {
                 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 @@ public class BlobStoreManagerService extends SystemService {
             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 @@ public class BlobStoreManagerService extends SystemService {
 
                 if (shouldRemove) {
                     blobMetadata.getBlobFile().delete();
-                    mKnownBlobIds.remove(blobMetadata.getBlobId());
+                    mActiveBlobIds.remove(blobMetadata.getBlobId());
                     deletedBlobIds.add(blobMetadata.getBlobId());
                 }
                 return shouldRemove;
@@ -848,7 +879,7 @@ public class BlobStoreManagerService extends SystemService {
 
                 if (shouldRemove) {
                     blobStoreSession.getSessionFile().delete();
-                    mKnownBlobIds.remove(blobStoreSession.getSessionId());
+                    mActiveBlobIds.remove(blobStoreSession.getSessionId());
                     indicesToRemove.add(j);
                     deletedBlobIds.add(blobStoreSession.getSessionId());
                 }
@@ -895,7 +926,7 @@ public class BlobStoreManagerService extends SystemService {
             }
             blobMetadata.getBlobFile().delete();
             userBlobs.remove(blobHandle);
-            mKnownBlobIds.remove(blobMetadata.getBlobId());
+            mActiveBlobIds.remove(blobMetadata.getBlobId());
             writeBlobsInfoAsync();
         }
     }