diff options
4 files changed, 56 insertions, 27 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..3d4154a227be 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java @@ -398,6 +398,26 @@ class BlobMetadata { return revocableFd.getRevocableFileDescriptor(); } + void destroy() { + revokeAllFds(); + getBlobFile().delete(); + } + + private void revokeAllFds() { + synchronized (mRevocableFds) { + for (int i = 0, pkgCount = mRevocableFds.size(); i < pkgCount; ++i) { + final ArraySet<RevocableFileDescriptor> packageFds = + mRevocableFds.valueAt(i); + if (packageFds == null) { + continue; + } + for (int j = 0, fdCount = packageFds.size(); j < fdCount; ++j) { + packageFds.valueAt(j).revoke(); + } + } + } + } + boolean shouldBeDeleted(boolean respectLeaseWaitTime) { // Expired data blobs if (getBlobHandle().isExpired()) { 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..f7468d8faa62 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java @@ -606,7 +606,11 @@ public class BlobStoreManagerService extends SystemService { UserHandle.getUserId(callingUid)); userBlobs.entrySet().removeIf(entry -> { final BlobMetadata blobMetadata = entry.getValue(); - return blobMetadata.getBlobId() == blobId; + if (blobMetadata.getBlobId() == blobId) { + deleteBlobLocked(blobMetadata); + return true; + } + return false; }); writeBlobsInfoAsync(); } @@ -657,11 +661,10 @@ public class BlobStoreManagerService extends SystemService { switch (session.getState()) { case STATE_ABANDONED: case STATE_VERIFIED_INVALID: - session.getSessionFile().delete(); synchronized (mBlobsLock) { + deleteSessionLocked(session); getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid())) .remove(session.getSessionId()); - mActiveBlobIds.remove(session.getSessionId()); if (LOGV) { Slog.v(TAG, "Session is invalid; deleted " + session); } @@ -682,8 +685,7 @@ public class BlobStoreManagerService extends SystemService { Slog.d(TAG, "Failed to commit: too many committed blobs. count: " + committedBlobsCount + "; blob: " + session); session.sendCommitCallbackResult(COMMIT_RESULT_ERROR); - session.getSessionFile().delete(); - mActiveBlobIds.remove(session.getSessionId()); + deleteSessionLocked(session); getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid())) .remove(session.getSessionId()); break; @@ -732,8 +734,7 @@ public class BlobStoreManagerService extends SystemService { } // Delete redundant data from recommits. if (session.getSessionId() != blob.getBlobId()) { - session.getSessionFile().delete(); - mActiveBlobIds.remove(session.getSessionId()); + deleteSessionLocked(session); } getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid())) .remove(session.getSessionId()); @@ -1019,8 +1020,7 @@ public class BlobStoreManagerService extends SystemService { userSessions.removeIf((sessionId, blobStoreSession) -> { if (blobStoreSession.getOwnerUid() == uid && blobStoreSession.getOwnerPackageName().equals(packageName)) { - blobStoreSession.getSessionFile().delete(); - mActiveBlobIds.remove(blobStoreSession.getSessionId()); + deleteSessionLocked(blobStoreSession); return true; } return false; @@ -1061,8 +1061,7 @@ public class BlobStoreManagerService extends SystemService { if (userSessions != null) { for (int i = 0, count = userSessions.size(); i < count; ++i) { final BlobStoreSession session = userSessions.valueAt(i); - session.getSessionFile().delete(); - mActiveBlobIds.remove(session.getSessionId()); + deleteSessionLocked(session); } } @@ -1138,8 +1137,7 @@ public class BlobStoreManagerService extends SystemService { } if (shouldRemove) { - blobStoreSession.getSessionFile().delete(); - mActiveBlobIds.remove(blobStoreSession.getSessionId()); + deleteSessionLocked(blobStoreSession); deletedBlobIds.add(blobStoreSession.getSessionId()); } return shouldRemove; @@ -1151,8 +1149,14 @@ public class BlobStoreManagerService extends SystemService { } @GuardedBy("mBlobsLock") + private void deleteSessionLocked(BlobStoreSession blobStoreSession) { + blobStoreSession.destroy(); + mActiveBlobIds.remove(blobStoreSession.getSessionId()); + } + + @GuardedBy("mBlobsLock") private void deleteBlobLocked(BlobMetadata blobMetadata) { - blobMetadata.getBlobFile().delete(); + blobMetadata.destroy(); mActiveBlobIds.remove(blobMetadata.getBlobId()); } diff --git a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java index 53e296ba3d11..2f83be1e0370 100644 --- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java +++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java @@ -479,6 +479,11 @@ class BlobStoreSession extends IBlobStoreSession.Stub { } } + void destroy() { + revokeAllFds(); + getSessionFile().delete(); + } + private void revokeAllFds() { synchronized (mRevocableFds) { for (int i = mRevocableFds.size() - 1; i >= 0; --i) { 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..4e2f9a495fe8 100644 --- a/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java @@ -174,10 +174,10 @@ public class BlobStoreManagerServiceTest { mService.handlePackageRemoved(TEST_PKG1, TEST_UID1); // Verify sessions are removed - verify(sessionFile1).delete(); - verify(sessionFile2, never()).delete(); - verify(sessionFile3, never()).delete(); - verify(sessionFile4).delete(); + verify(session1).destroy(); + verify(session2, never()).destroy(); + verify(session3, never()).destroy(); + verify(session4).destroy(); assertThat(mUserSessions.size()).isEqualTo(2); assertThat(mUserSessions.get(sessionId1)).isNull(); @@ -193,9 +193,9 @@ public class BlobStoreManagerServiceTest { verify(blobMetadata3).removeCommitter(TEST_PKG1, TEST_UID1); verify(blobMetadata3).removeLeasee(TEST_PKG1, TEST_UID1); - verify(blobFile1, never()).delete(); - verify(blobFile2).delete(); - verify(blobFile3).delete(); + verify(blobMetadata1, never()).destroy(); + verify(blobMetadata2).destroy(); + verify(blobMetadata3).destroy(); assertThat(mUserBlobs.size()).isEqualTo(1); assertThat(mUserBlobs.get(blobHandle1)).isNotNull(); @@ -272,9 +272,9 @@ public class BlobStoreManagerServiceTest { mService.handleIdleMaintenanceLocked(); // Verify stale sessions are removed - verify(sessionFile1).delete(); - verify(sessionFile2, never()).delete(); - verify(sessionFile3).delete(); + verify(session1).destroy(); + verify(session2, never()).destroy(); + verify(session3).destroy(); assertThat(mUserSessions.size()).isEqualTo(1); assertThat(mUserSessions.get(sessionId2)).isNotNull(); @@ -317,9 +317,9 @@ public class BlobStoreManagerServiceTest { mService.handleIdleMaintenanceLocked(); // Verify stale blobs are removed - verify(blobFile1).delete(); - verify(blobFile2, never()).delete(); - verify(blobFile3).delete(); + verify(blobMetadata1).destroy(); + verify(blobMetadata2, never()).destroy(); + verify(blobMetadata3).destroy(); assertThat(mUserBlobs.size()).isEqualTo(1); assertThat(mUserBlobs.get(blobHandle2)).isNotNull(); |