Merge "Delete a blob after the last lease of it is released." into rvc-dev

3 files changed, 100 insertions, 46 deletions

diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
index 4a1c63e1e264..e5a685f15df8 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
@@ -32,6 +32,7 @@ import static android.system.OsConstants.O_RDONLY;
 import static com.android.server.blob.BlobStoreConfig.TAG;
 import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ADD_DESC_RES_NAME;
 import static com.android.server.blob.BlobStoreConfig.XML_VERSION_ADD_STRING_DESC;
+import static com.android.server.blob.BlobStoreConfig.hasLeaseWaitTimeElapsed;
 import static com.android.server.blob.BlobStoreUtils.getDescriptionResourceId;
 import static com.android.server.blob.BlobStoreUtils.getPackageResources;
 
@@ -227,6 +228,35 @@ class BlobMetadata {
         return false;
     }
 
+    boolean isACommitter(@NonNull String packageName, int uid) {
+        synchronized (mMetadataLock) {
+            return isAnAccessor(mCommitters, packageName, uid);
+        }
+    }
+
+    boolean isALeasee(@Nullable String packageName, int uid) {
+        synchronized (mMetadataLock) {
+            return isAnAccessor(mLeasees, packageName, uid);
+        }
+    }
+
+    private static <T extends Accessor> boolean isAnAccessor(@NonNull ArraySet<T> accessors,
+            @Nullable String packageName, int uid) {
+        // Check if the package is an accessor of the data blob.
+        for (int i = 0, size = accessors.size(); i < size; ++i) {
+            final Accessor accessor = accessors.valueAt(i);
+            if (packageName != null && uid != INVALID_UID
+                    && accessor.equals(packageName, uid)) {
+                return true;
+            } else if (packageName != null && accessor.packageName.equals(packageName)) {
+                return true;
+            } else if (uid != INVALID_UID && accessor.uid == uid) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     boolean isALeasee(@NonNull String packageName) {
         return isALeasee(packageName, INVALID_UID);
     }
@@ -243,24 +273,6 @@ class BlobMetadata {
         return hasOtherLeasees(null, uid);
     }
 
-    boolean isALeasee(@Nullable String packageName, int uid) {
-        synchronized (mMetadataLock) {
-            // Check if the package is a leasee of the data blob.
-            for (int i = 0, size = mLeasees.size(); i < size; ++i) {
-                final Leasee leasee = mLeasees.valueAt(i);
-                if (packageName != null && uid != INVALID_UID
-                        && leasee.equals(packageName, uid)) {
-                    return true;
-                } else if (packageName != null && leasee.packageName.equals(packageName)) {
-                    return true;
-                } else if (uid != INVALID_UID && leasee.uid == uid) {
-                    return true;
-                }
-            }
-        }
-        return false;
-    }
-
     private boolean hasOtherLeasees(@Nullable String packageName, int uid) {
         synchronized (mMetadataLock) {
             if (mCommitters.size() > 1 || mLeasees.size() > 1) {
@@ -355,6 +367,22 @@ class BlobMetadata {
         return revocableFd.getRevocableFileDescriptor();
     }
 
+    boolean shouldBeDeleted(boolean respectLeaseWaitTime) {
+        // Expired data blobs
+        if (getBlobHandle().isExpired()) {
+            return true;
+        }
+
+        // Blobs with no active leases
+        // TODO: Track commit time instead of using last modified time.
+        if ((!respectLeaseWaitTime || hasLeaseWaitTimeElapsed(getBlobFile().lastModified()))
+                && !hasLeases()) {
+            return true;
+        }
+
+        return false;
+    }
+
     void dump(IndentingPrintWriter fout, DumpArgs dumpArgs) {
         fout.println("blobHandle:");
         fout.increaseIndent();
diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java
index 5e8ea7afaec7..f2c158658562 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreConfig.java
@@ -29,6 +29,7 @@ import android.provider.DeviceConfig.Properties;
 import android.util.DataUnit;
 import android.util.Log;
 import android.util.Slog;
+import android.util.TimeUtils;
 
 import com.android.internal.util.IndentingPrintWriter;
 
@@ -88,6 +89,17 @@ class BlobStoreConfig {
         public static float TOTAL_BYTES_PER_APP_LIMIT_FRACTION =
                 DEFAULT_TOTAL_BYTES_PER_APP_LIMIT_FRACTION;
 
+        /**
+         * Denotes the duration from the time a blob is committed that we wait for a lease to
+         * be acquired before deciding to delete the blob for having no leases.
+         */
+        public static final String KEY_LEASE_ACQUISITION_WAIT_DURATION_MS =
+                "lease_acquisition_wait_time_ms";
+        public static final long DEFAULT_LEASE_ACQUISITION_WAIT_DURATION_MS =
+                TimeUnit.HOURS.toMillis(6);
+        public static long LEASE_ACQUISITION_WAIT_DURATION_MS =
+                DEFAULT_LEASE_ACQUISITION_WAIT_DURATION_MS;
+
         static void refresh(Properties properties) {
             if (!NAMESPACE_BLOBSTORE.equals(properties.getNamespace())) {
                 return;
@@ -102,6 +114,10 @@ class BlobStoreConfig {
                         TOTAL_BYTES_PER_APP_LIMIT_FRACTION = properties.getFloat(key,
                                 DEFAULT_TOTAL_BYTES_PER_APP_LIMIT_FRACTION);
                         break;
+                    case KEY_LEASE_ACQUISITION_WAIT_DURATION_MS:
+                        LEASE_ACQUISITION_WAIT_DURATION_MS = properties.getLong(key,
+                                DEFAULT_LEASE_ACQUISITION_WAIT_DURATION_MS);
+                        break;
                     default:
                         Slog.wtf(TAG, "Unknown key in device config properties: " + key);
                 }
@@ -117,6 +133,9 @@ class BlobStoreConfig {
             fout.println(String.format(dumpFormat, KEY_TOTAL_BYTES_PER_APP_LIMIT_FRACTION,
                     TOTAL_BYTES_PER_APP_LIMIT_FRACTION,
                     DEFAULT_TOTAL_BYTES_PER_APP_LIMIT_FRACTION));
+            fout.println(String.format(dumpFormat, KEY_LEASE_ACQUISITION_WAIT_DURATION_MS,
+                    TimeUtils.formatDuration(LEASE_ACQUISITION_WAIT_DURATION_MS),
+                    TimeUtils.formatDuration(DEFAULT_LEASE_ACQUISITION_WAIT_DURATION_MS)));
         }
     }
 
@@ -136,6 +155,14 @@ class BlobStoreConfig {
         return Math.max(DeviceConfigProperties.TOTAL_BYTES_PER_APP_LIMIT_FLOOR, totalBytesLimit);
     }
 
+    /**
+     * Returns whether the wait time for lease acquisition for a blob has elapsed.
+     */
+    public static boolean hasLeaseWaitTimeElapsed(long commitTimeMs) {
+        return commitTimeMs + DeviceConfigProperties.LEASE_ACQUISITION_WAIT_DURATION_MS
+                < System.currentTimeMillis();
+    }
+
     @Nullable
     public static File prepareBlobFile(long sessionId) {
         final File blobsDir = prepareBlobsDir();
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 f4b8f0f39e85..15c069fbd007 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
@@ -424,8 +424,9 @@ public class BlobStoreManagerService extends SystemService {
     private void releaseLeaseInternal(BlobHandle blobHandle, int callingUid,
             String callingPackage) {
         synchronized (mBlobsLock) {
-            final BlobMetadata blobMetadata = getUserBlobsLocked(UserHandle.getUserId(callingUid))
-                    .get(blobHandle);
+            final ArrayMap<BlobHandle, BlobMetadata> userBlobs =
+                    getUserBlobsLocked(UserHandle.getUserId(callingUid));
+            final BlobMetadata blobMetadata = userBlobs.get(blobHandle);
             if (blobMetadata == null || !blobMetadata.isAccessAllowedForCaller(
                     callingPackage, callingUid)) {
                 throw new SecurityException("Caller not allowed to access " + blobHandle
@@ -436,6 +437,10 @@ public class BlobStoreManagerService extends SystemService {
                 Slog.v(TAG, "Released lease on " + blobHandle
                         + "; callingUid=" + callingUid + ", callingPackage=" + callingPackage);
             }
+            if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) {
+                deleteBlobLocked(blobMetadata);
+                userBlobs.remove(blobHandle);
+            }
             writeBlobsInfoAsync();
         }
     }
@@ -863,12 +868,15 @@ public class BlobStoreManagerService extends SystemService {
                     getUserBlobsLocked(UserHandle.getUserId(uid));
             userBlobs.entrySet().removeIf(entry -> {
                 final BlobMetadata blobMetadata = entry.getValue();
-                blobMetadata.removeCommitter(packageName, uid);
+                final boolean isACommitter = blobMetadata.isACommitter(packageName, uid);
+                if (isACommitter) {
+                    blobMetadata.removeCommitter(packageName, uid);
+                }
                 blobMetadata.removeLeasee(packageName, uid);
-                // Delete the blob if it doesn't have any active leases.
-                if (!blobMetadata.hasLeases()) {
-                    blobMetadata.getBlobFile().delete();
-                    mActiveBlobIds.remove(blobMetadata.getBlobId());
+                // Regardless of when the blob is committed, we need to delete
+                // it if it was from the deleted package to ensure we delete all traces of it.
+                if (blobMetadata.shouldBeDeleted(isACommitter /* respectLeaseWaitTime */)) {
+                    deleteBlobLocked(blobMetadata);
                     return true;
                 }
                 return false;
@@ -899,8 +907,7 @@ public class BlobStoreManagerService extends SystemService {
             if (userBlobs != null) {
                 for (int i = 0, count = userBlobs.size(); i < count; ++i) {
                     final BlobMetadata blobMetadata = userBlobs.valueAt(i);
-                    blobMetadata.getBlobFile().delete();
-                    mActiveBlobIds.remove(blobMetadata.getBlobId());
+                    deleteBlobLocked(blobMetadata);
                 }
             }
             if (LOGV) {
@@ -938,27 +945,14 @@ public class BlobStoreManagerService extends SystemService {
         for (int i = 0, userCount = mBlobsMap.size(); i < userCount; ++i) {
             final ArrayMap<BlobHandle, BlobMetadata> userBlobs = mBlobsMap.valueAt(i);
             userBlobs.entrySet().removeIf(entry -> {
-                final BlobHandle blobHandle = entry.getKey();
                 final BlobMetadata blobMetadata = entry.getValue();
-                boolean shouldRemove = false;
-
-                // Cleanup expired data blobs.
-                if (blobHandle.isExpired()) {
-                    shouldRemove = true;
-                }
 
-                // Cleanup blobs with no active leases.
-                // TODO: Exclude blobs which were just committed.
-                if (!blobMetadata.hasLeases()) {
-                    shouldRemove = true;
-                }
-
-                if (shouldRemove) {
-                    blobMetadata.getBlobFile().delete();
-                    mActiveBlobIds.remove(blobMetadata.getBlobId());
+                if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) {
+                    deleteBlobLocked(blobMetadata);
                     deletedBlobIds.add(blobMetadata.getBlobId());
+                    return true;
                 }
-                return shouldRemove;
+                return false;
             });
         }
         writeBlobsInfoAsync();
@@ -995,6 +989,12 @@ public class BlobStoreManagerService extends SystemService {
         writeBlobSessionsAsync();
     }
 
+    @GuardedBy("mBlobsLock")
+    private void deleteBlobLocked(BlobMetadata blobMetadata) {
+        blobMetadata.getBlobFile().delete();
+        mActiveBlobIds.remove(blobMetadata.getBlobId());
+    }
+
     void runClearAllSessions(@UserIdInt int userId) {
         synchronized (mBlobsLock) {
             if (userId == UserHandle.USER_ALL) {
@@ -1024,9 +1024,8 @@ public class BlobStoreManagerService extends SystemService {
             if (blobMetadata == null) {
                 return;
             }
-            blobMetadata.getBlobFile().delete();
+            deleteBlobLocked(blobMetadata);
             userBlobs.remove(blobHandle);
-            mActiveBlobIds.remove(blobMetadata.getBlobId());
             writeBlobsInfoAsync();
         }
     }