Merge "Revoke any open fds when deleting a session/blob." into rvc-dev (d06ebaee) · Commits · e / os / android_frameworks_base

apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java

+20 −0

Original line number	Diff line number	Diff line
		@@ -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()) {

apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java

+18 −14

Original line number	Diff line number	Diff line
		@@ -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;
		@@ -1150,9 +1148,15 @@ public class BlobStoreManagerService extends SystemService {
		writeBlobSessionsAsync();
		}

		@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());
		}

apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java

+5 −0

Original line number	Diff line number	Diff line
		@@ -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) {

services/tests/mockingservicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java

+13 −13

Original line number	Diff line number	Diff line
		@@ -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();