Merge "Revoke any open fds when deleting a session/blob." into rvc-dev am: d06ebaeeed am: 2b6e4fd231 am: 5a9309a787 am: be0f671443
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11990986
Change-Id: I71bb110dbe850990cd2e9829b2a5c6db0511dd93
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 4d29045..3d4154a2 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 @@
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 7a6c884..f7468d8 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 @@
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 @@
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 @@
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 @@
}
// 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 @@
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 @@
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 @@
}
if (shouldRemove) {
- blobStoreSession.getSessionFile().delete();
- mActiveBlobIds.remove(blobStoreSession.getSessionId());
+ deleteSessionLocked(blobStoreSession);
deletedBlobIds.add(blobStoreSession.getSessionId());
}
return shouldRemove;
@@ -1151,8 +1149,14 @@
}
@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 53e296b..2f83be1 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 @@
}
}
+ 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 7446289..4e2f9a4 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 @@
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 @@
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 @@
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 @@
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();