diff options
3 files changed, 49 insertions, 23 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 4d29045fa631..5c3e4170391d 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java @@ -154,10 +154,10 @@ class BlobMetadata { } } - void removeInvalidCommitters(SparseArray<String> packages) { + void removeCommittersFromUnknownPkgs(SparseArray<String> knownPackages) { synchronized (mMetadataLock) { mCommitters.removeIf(committer -> - !committer.packageName.equals(packages.get(committer.uid))); + !committer.packageName.equals(knownPackages.get(committer.uid))); } } @@ -200,16 +200,27 @@ class BlobMetadata { } } - void removeInvalidLeasees(SparseArray<String> packages) { + void removeLeaseesFromUnknownPkgs(SparseArray<String> knownPackages) { synchronized (mMetadataLock) { mLeasees.removeIf(leasee -> - !leasee.packageName.equals(packages.get(leasee.uid))); + !leasee.packageName.equals(knownPackages.get(leasee.uid))); } } - boolean hasLeases() { + void removeExpiredLeases() { synchronized (mMetadataLock) { - return !mLeasees.isEmpty(); + mLeasees.removeIf(leasee -> !leasee.isStillValid()); + } + } + + boolean hasValidLeases() { + synchronized (mMetadataLock) { + for (int i = 0, size = mLeasees.size(); i < size; ++i) { + if (mLeasees.valueAt(i).isStillValid()) { + return true; + } + } + return false; } } @@ -226,8 +237,7 @@ class BlobMetadata { // Check if packageName already holds a lease on the blob. for (int i = 0, size = mLeasees.size(); i < size; ++i) { final Leasee leasee = mLeasees.valueAt(i); - if (leasee.equals(callingPackage, callingUid) - && leasee.isStillValid()) { + if (leasee.isStillValid() && leasee.equals(callingPackage, callingUid)) { return true; } } @@ -259,25 +269,32 @@ class BlobMetadata { boolean isALeasee(@Nullable String packageName, int uid) { synchronized (mMetadataLock) { - return isAnAccessor(mLeasees, packageName, uid); + final Leasee leasee = getAccessor(mLeasees, packageName, uid); + return leasee != null && leasee.isStillValid(); } } 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. + return getAccessor(accessors, packageName, uid) != null; + } + + private static <T extends Accessor> T getAccessor(@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; + return (T) accessor; } else if (packageName != null && accessor.packageName.equals(packageName)) { - return true; + return (T) accessor; } else if (uid != INVALID_UID && accessor.uid == uid) { - return true; + return (T) accessor; } } - return false; + return null; } boolean isALeasee(@NonNull String packageName) { @@ -298,11 +315,11 @@ class BlobMetadata { private boolean hasOtherLeasees(@Nullable String packageName, int uid) { synchronized (mMetadataLock) { - if (mCommitters.size() > 1 || mLeasees.size() > 1) { - return true; - } for (int i = 0, size = mLeasees.size(); i < size; ++i) { final Leasee leasee = mLeasees.valueAt(i); + if (!leasee.isStillValid()) { + continue; + } // TODO: Also exclude packages which are signed with same cert? if (packageName != null && uid != INVALID_UID && !leasee.equals(packageName, uid)) { @@ -322,6 +339,9 @@ class BlobMetadata { synchronized (mMetadataLock) { for (int i = 0, size = mLeasees.size(); i < size; ++i) { final Leasee leasee = mLeasees.valueAt(i); + if (!leasee.isStillValid()) { + continue; + } if (leasee.uid == uid && leasee.packageName.equals(packageName)) { final int descriptionResId = leasee.descriptionResEntryName == null ? Resources.ID_NULL @@ -406,7 +426,7 @@ class BlobMetadata { // Blobs with no active leases if ((!respectLeaseWaitTime || hasLeaseWaitTimeElapsedForAll()) - && !hasLeases()) { + && !hasValidLeases()) { return true; } @@ -695,7 +715,7 @@ class BlobMetadata { } boolean isStillValid() { - return expiryTimeMillis == 0 || expiryTimeMillis <= System.currentTimeMillis(); + return expiryTimeMillis == 0 || expiryTimeMillis >= System.currentTimeMillis(); } void dump(@NonNull Context context, @NonNull IndentingPrintWriter fout) { 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 7a6c884848d8..d75f46dde281 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java @@ -539,7 +539,7 @@ public class BlobStoreManagerService extends SystemService { Slog.v(TAG, "Released lease on " + blobHandle + "; callingUid=" + callingUid + ", callingPackage=" + callingPackage); } - if (!blobMetadata.hasLeases()) { + if (!blobMetadata.hasValidLeases()) { mHandler.postDelayed(() -> { synchronized (mBlobsLock) { // Check if blobMetadata object is still valid. If it is not, then @@ -583,6 +583,9 @@ public class BlobStoreManagerService extends SystemService { getUserBlobsLocked(userId).forEach((blobHandle, blobMetadata) -> { final ArrayList<LeaseInfo> leaseInfos = new ArrayList<>(); blobMetadata.forEachLeasee(leasee -> { + if (!leasee.isStillValid()) { + return; + } final int descriptionResId = leasee.descriptionResEntryName == null ? Resources.ID_NULL : getDescriptionResourceId(resourcesGetter.apply(leasee.packageName), @@ -921,8 +924,8 @@ public class BlobStoreManagerService extends SystemService { blobMetadata.getBlobFile().delete(); } else { addBlobForUserLocked(blobMetadata, blobMetadata.getUserId()); - blobMetadata.removeInvalidCommitters(userPackages); - blobMetadata.removeInvalidLeasees(userPackages); + blobMetadata.removeCommittersFromUnknownPkgs(userPackages); + blobMetadata.removeLeaseesFromUnknownPkgs(userPackages); } mCurrentMaxSessionId = Math.max(mCurrentMaxSessionId, blobMetadata.getBlobId()); } @@ -1111,6 +1114,9 @@ public class BlobStoreManagerService extends SystemService { userBlobs.entrySet().removeIf(entry -> { final BlobMetadata blobMetadata = entry.getValue(); + // Remove expired leases + blobMetadata.removeExpiredLeases(); + if (blobMetadata.shouldBeDeleted(true /* respectLeaseWaitTime */)) { deleteBlobLocked(blobMetadata); deletedBlobIds.add(blobMetadata.getBlobId()); 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 7446289cd498..37e29b7bf18d 100644 --- a/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java @@ -377,11 +377,11 @@ public class BlobStoreManagerServiceTest { } private BlobMetadata createBlobMetadataMock(long blobId, File blobFile, - BlobHandle blobHandle, boolean hasLeases) { + BlobHandle blobHandle, boolean hasValidLeases) { final BlobMetadata blobMetadata = mock(BlobMetadata.class); doReturn(blobId).when(blobMetadata).getBlobId(); doReturn(blobFile).when(blobMetadata).getBlobFile(); - doReturn(hasLeases).when(blobMetadata).hasLeases(); + doReturn(hasValidLeases).when(blobMetadata).hasValidLeases(); doReturn(blobHandle).when(blobMetadata).getBlobHandle(); doCallRealMethod().when(blobMetadata).shouldBeDeleted(anyBoolean()); doReturn(true).when(blobMetadata).hasLeaseWaitTimeElapsedForAll(); |