Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 507d02cf authored by Sudheer Shanka's avatar Sudheer Shanka Committed by Automerger Merge Worker
Browse files

Merge "Revoke any open fds when deleting a session/blob." into rvc-dev am: d06ebaee 

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11990986

Change-Id: I8ce2545ef44519388a3f0667a3249484977631c8
parents 245aa49c d06ebaee

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