Delete blobs if there are no active leases on them.
+ Fix a bug in how sessions belonging to uninstalled pkgs
are removed.
Bug: 148637068
Test: atest services/tests/servicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java
Change-Id: I98cd9e974fb1a46bb5868a644b1a0d98ff61eb57
diff --git a/apex/blobstore/TEST_MAPPING b/apex/blobstore/TEST_MAPPING
index 4dc0c49..cfe19a5 100644
--- a/apex/blobstore/TEST_MAPPING
+++ b/apex/blobstore/TEST_MAPPING
@@ -2,6 +2,14 @@
"presubmit": [
{
"name": "CtsBlobStoreTestCases"
+ },
+ {
+ "name": "FrameworksServicesTests",
+ "options": [
+ {
+ "include-filter": "com.android.server.blob"
+ }
+ ]
}
]
}
\ No newline at end of file
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 e9838d6..7052d60 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobMetadata.java
@@ -156,6 +156,12 @@
}
}
+ boolean hasLeases() {
+ synchronized (mMetadataLock) {
+ return mLeasees.isEmpty();
+ }
+ }
+
boolean isAccessAllowedForCaller(String callingPackage, int callingUid) {
// TODO: verify blob is still valid (expiryTime is not elapsed)
synchronized (mMetadataLock) {
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 fcc30e30..f10319a 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreManagerService.java
@@ -67,6 +67,7 @@
import android.util.Xml;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.FastXmlSerializer;
import com.android.internal.util.IndentingPrintWriter;
@@ -112,20 +113,32 @@
private final Context mContext;
private final Handler mHandler;
+ private final Injector mInjector;
private final SessionStateChangeListener mSessionStateChangeListener =
new SessionStateChangeListener();
private PackageManagerInternal mPackageManagerInternal;
public BlobStoreManagerService(Context context) {
- super(context);
- mContext = context;
+ this(context, new Injector());
+ }
+ @VisibleForTesting
+ BlobStoreManagerService(Context context, Injector injector) {
+ super(context);
+
+ mContext = context;
+ mInjector = injector;
+ mHandler = mInjector.initializeMessageHandler();
+ }
+
+ private static Handler initializeMessageHandler() {
final HandlerThread handlerThread = new ServiceThread(TAG,
Process.THREAD_PRIORITY_BACKGROUND, true /* allowIo */);
handlerThread.start();
- mHandler = new Handler(handlerThread.getLooper());
- Watchdog.getInstance().addThread(mHandler);
+ final Handler handler = new Handler(handlerThread.getLooper());
+ Watchdog.getInstance().addThread(handler);
+ return handler;
}
@Override
@@ -181,6 +194,20 @@
return userBlobs;
}
+ @VisibleForTesting
+ void addUserSessionsForTest(LongSparseArray<BlobStoreSession> userSessions, int userId) {
+ synchronized (mBlobsLock) {
+ mSessions.put(userId, userSessions);
+ }
+ }
+
+ @VisibleForTesting
+ void addUserBlobsForTest(ArrayMap<BlobHandle, BlobMetadata> userBlobs, int userId) {
+ synchronized (mBlobsLock) {
+ mBlobsMap.put(userId, userBlobs);
+ }
+ }
+
private long createSessionInternal(BlobHandle blobHandle,
int callingUid, String callingPackage) {
synchronized (mBlobsLock) {
@@ -293,23 +320,23 @@
case STATE_ABANDONED:
case STATE_VERIFIED_INVALID:
session.getSessionFile().delete();
- getUserSessionsLocked(UserHandle.getUserId(session.ownerUid))
- .remove(session.sessionId);
+ getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid()))
+ .remove(session.getSessionId());
break;
case STATE_COMMITTED:
session.verifyBlobData();
break;
case STATE_VERIFIED_VALID:
- final int userId = UserHandle.getUserId(session.ownerUid);
+ final int userId = UserHandle.getUserId(session.getOwnerUid());
final ArrayMap<BlobHandle, BlobMetadata> userBlobs = getUserBlobsLocked(userId);
- BlobMetadata blob = userBlobs.get(session.blobHandle);
+ BlobMetadata blob = userBlobs.get(session.getBlobHandle());
if (blob == null) {
blob = new BlobMetadata(mContext,
- session.sessionId, session.blobHandle, userId);
- userBlobs.put(session.blobHandle, blob);
+ session.getSessionId(), session.getBlobHandle(), userId);
+ userBlobs.put(session.getBlobHandle(), blob);
}
- final Committer newCommitter = new Committer(session.ownerPackageName,
- session.ownerUid, session.getBlobAccessMode());
+ final Committer newCommitter = new Committer(session.getOwnerPackageName(),
+ session.getOwnerUid(), session.getBlobAccessMode());
final Committer existingCommitter = blob.getExistingCommitter(newCommitter);
blob.addCommitter(newCommitter);
try {
@@ -319,8 +346,8 @@
blob.addCommitter(existingCommitter);
session.sendCommitCallbackResult(COMMIT_RESULT_ERROR);
}
- getUserSessionsLocked(UserHandle.getUserId(session.ownerUid))
- .remove(session.sessionId);
+ getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid()))
+ .remove(session.getSessionId());
break;
default:
Slog.wtf(TAG, "Invalid session state: "
@@ -399,17 +426,17 @@
continue;
}
final SparseArray<String> userPackages = allPackages.get(
- UserHandle.getUserId(session.ownerUid));
+ UserHandle.getUserId(session.getOwnerUid()));
if (userPackages != null
- && session.ownerPackageName.equals(
- userPackages.get(session.ownerUid))) {
- getUserSessionsLocked(UserHandle.getUserId(session.ownerUid)).put(
- session.sessionId, session);
+ && session.getOwnerPackageName().equals(
+ userPackages.get(session.getOwnerUid()))) {
+ getUserSessionsLocked(UserHandle.getUserId(session.getOwnerUid())).put(
+ session.getSessionId(), session);
} else {
// Unknown package or the session data does not belong to this package.
session.getSessionFile().delete();
}
- mCurrentMaxSessionId = Math.max(mCurrentMaxSessionId, session.sessionId);
+ mCurrentMaxSessionId = Math.max(mCurrentMaxSessionId, session.getSessionId());
}
}
} catch (Exception e) {
@@ -568,7 +595,7 @@
return new AtomicFile(file, "blobs_index" /* commitLogTag */);
}
- private void handlePackageRemoved(String packageName, int uid) {
+ void handlePackageRemoved(String packageName, int uid) {
synchronized (mBlobsLock) {
// Clean up any pending sessions
final LongSparseArray<BlobStoreSession> userSessions =
@@ -576,25 +603,35 @@
final ArrayList<Integer> indicesToRemove = new ArrayList<>();
for (int i = 0, count = userSessions.size(); i < count; ++i) {
final BlobStoreSession session = userSessions.valueAt(i);
- if (session.ownerUid == uid
- && session.ownerPackageName.equals(packageName)) {
+ if (session.getOwnerUid() == uid
+ && session.getOwnerPackageName().equals(packageName)) {
session.getSessionFile().delete();
indicesToRemove.add(i);
}
}
for (int i = 0, count = indicesToRemove.size(); i < count; ++i) {
- userSessions.removeAt(i);
+ userSessions.removeAt(indicesToRemove.get(i));
}
+ writeBlobSessionsAsync();
// Remove the package from the committer and leasee list
final ArrayMap<BlobHandle, BlobMetadata> userBlobs =
getUserBlobsLocked(UserHandle.getUserId(uid));
+ indicesToRemove.clear();
for (int i = 0, count = userBlobs.size(); i < count; ++i) {
final BlobMetadata blobMetadata = userBlobs.valueAt(i);
blobMetadata.removeCommitter(packageName, uid);
blobMetadata.removeLeasee(packageName, uid);
+ // Delete the blob if it doesn't have any active leases.
+ if (!blobMetadata.hasLeases()) {
+ blobMetadata.getBlobFile().delete();
+ indicesToRemove.add(i);
+ }
}
- // TODO: clean-up blobs which doesn't have any active leases.
+ for (int i = 0, count = indicesToRemove.size(); i < count; ++i) {
+ userBlobs.removeAt(indicesToRemove.get(i));
+ }
+ writeBlobsInfoAsync();
}
}
@@ -809,4 +846,11 @@
}
}
}
+
+ @VisibleForTesting
+ static class Injector {
+ public Handler initializeMessageHandler() {
+ return BlobStoreManagerService.initializeMessageHandler();
+ }
+ }
}
\ No newline at end of file
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 7d1c166..e6947d4 100644
--- a/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
+++ b/apex/blobstore/service/java/com/android/server/blob/BlobStoreSession.java
@@ -47,6 +47,7 @@
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.IndentingPrintWriter;
import com.android.internal.util.Preconditions;
import com.android.internal.util.XmlUtils;
@@ -64,7 +65,8 @@
import java.util.Arrays;
/** TODO: add doc */
-public class BlobStoreSession extends IBlobStoreSession.Stub {
+@VisibleForTesting
+class BlobStoreSession extends IBlobStoreSession.Stub {
static final int STATE_OPENED = 1;
static final int STATE_CLOSED = 0;
@@ -78,10 +80,10 @@
private final Context mContext;
private final SessionStateChangeListener mListener;
- public final BlobHandle blobHandle;
- public final long sessionId;
- public final int ownerUid;
- public final String ownerPackageName;
+ private final BlobHandle mBlobHandle;
+ private final long mSessionId;
+ private final int mOwnerUid;
+ private final String mOwnerPackageName;
// Do not access this directly, instead use getSessionFile().
private File mSessionFile;
@@ -101,15 +103,31 @@
BlobStoreSession(Context context, long sessionId, BlobHandle blobHandle,
int ownerUid, String ownerPackageName, SessionStateChangeListener listener) {
this.mContext = context;
- this.blobHandle = blobHandle;
- this.sessionId = sessionId;
- this.ownerUid = ownerUid;
- this.ownerPackageName = ownerPackageName;
+ this.mBlobHandle = blobHandle;
+ this.mSessionId = sessionId;
+ this.mOwnerUid = ownerUid;
+ this.mOwnerPackageName = ownerPackageName;
this.mListener = listener;
}
+ public BlobHandle getBlobHandle() {
+ return mBlobHandle;
+ }
+
+ public long getSessionId() {
+ return mSessionId;
+ }
+
+ public int getOwnerUid() {
+ return mOwnerUid;
+ }
+
+ public String getOwnerPackageName() {
+ return mOwnerPackageName;
+ }
+
boolean hasAccess(int callingUid, String callingPackageName) {
- return ownerUid == callingUid && ownerPackageName.equals(callingPackageName);
+ return mOwnerUid == callingUid && mOwnerPackageName.equals(callingPackageName);
}
void open() {
@@ -357,12 +375,12 @@
void verifyBlobData() {
byte[] actualDigest = null;
try {
- actualDigest = FileUtils.digest(getSessionFile(), blobHandle.algorithm);
+ actualDigest = FileUtils.digest(getSessionFile(), mBlobHandle.algorithm);
} catch (IOException | NoSuchAlgorithmException e) {
Slog.e(TAG, "Error computing the digest", e);
}
synchronized (mSessionLock) {
- if (actualDigest != null && Arrays.equals(actualDigest, blobHandle.digest)) {
+ if (actualDigest != null && Arrays.equals(actualDigest, mBlobHandle.digest)) {
mState = STATE_VERIFIED_VALID;
// Commit callback will be sent once the data is persisted.
} else {
@@ -401,7 +419,7 @@
@Nullable
File getSessionFile() {
if (mSessionFile == null) {
- mSessionFile = BlobStoreConfig.prepareBlobFile(sessionId);
+ mSessionFile = BlobStoreConfig.prepareBlobFile(mSessionId);
}
return mSessionFile;
}
@@ -425,20 +443,20 @@
private void assertCallerIsOwner() {
final int callingUid = Binder.getCallingUid();
- if (callingUid != ownerUid) {
- throw new SecurityException(ownerUid + " is not the session owner");
+ if (callingUid != mOwnerUid) {
+ throw new SecurityException(mOwnerUid + " is not the session owner");
}
}
void dump(IndentingPrintWriter fout) {
synchronized (mSessionLock) {
fout.println("state: " + stateToString(mState));
- fout.println("ownerUid: " + ownerUid);
- fout.println("ownerPkg: " + ownerPackageName);
+ fout.println("ownerUid: " + mOwnerUid);
+ fout.println("ownerPkg: " + mOwnerPackageName);
fout.println("blobHandle:");
fout.increaseIndent();
- blobHandle.dump(fout);
+ mBlobHandle.dump(fout);
fout.decreaseIndent();
fout.println("accessMode:");
@@ -452,12 +470,12 @@
void writeToXml(@NonNull XmlSerializer out) throws IOException {
synchronized (mSessionLock) {
- XmlUtils.writeLongAttribute(out, ATTR_ID, sessionId);
- XmlUtils.writeStringAttribute(out, ATTR_PACKAGE, ownerPackageName);
- XmlUtils.writeIntAttribute(out, ATTR_UID, ownerUid);
+ XmlUtils.writeLongAttribute(out, ATTR_ID, mSessionId);
+ XmlUtils.writeStringAttribute(out, ATTR_PACKAGE, mOwnerPackageName);
+ XmlUtils.writeIntAttribute(out, ATTR_UID, mOwnerUid);
out.startTag(null, TAG_BLOB_HANDLE);
- blobHandle.writeToXml(out);
+ mBlobHandle.writeToXml(out);
out.endTag(null, TAG_BLOB_HANDLE);
out.startTag(null, TAG_ACCESS_MODE);
diff --git a/services/tests/servicestests/Android.bp b/services/tests/servicestests/Android.bp
index 556f636..fdaf484 100644
--- a/services/tests/servicestests/Android.bp
+++ b/services/tests/servicestests/Android.bp
@@ -45,6 +45,7 @@
"service-appsearch",
"service-jobscheduler",
"service-permission",
+ "service-blobstore",
// TODO: remove once Android migrates to JUnit 4.12,
// which provides assertThrows
"testng",
diff --git a/services/tests/servicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java
new file mode 100644
index 0000000..ff728e7
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/blob/BlobStoreManagerServiceTest.java
@@ -0,0 +1,189 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.server.blob;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import android.app.blob.BlobHandle;
+import android.content.Context;
+import android.os.Handler;
+import android.os.Looper;
+import android.os.Message;
+import android.os.UserHandle;
+import android.platform.test.annotations.Presubmit;
+import android.util.ArrayMap;
+import android.util.LongSparseArray;
+
+import androidx.test.InstrumentationRegistry;
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import com.android.server.blob.BlobStoreManagerService.Injector;
+import com.android.server.blob.BlobStoreManagerService.SessionStateChangeListener;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+@Presubmit
+public class BlobStoreManagerServiceTest {
+ private Context mContext;
+ private Handler mHandler;
+ private BlobStoreManagerService mService;
+
+ private LongSparseArray<BlobStoreSession> mUserSessions;
+ private ArrayMap<BlobHandle, BlobMetadata> mUserBlobs;
+
+ private SessionStateChangeListener mStateChangeListener;
+
+ private static final String TEST_PKG1 = "com.example1";
+ private static final String TEST_PKG2 = "com.example2";
+ private static final String TEST_PKG3 = "com.example3";
+
+ private static final int TEST_UID1 = 10001;
+ private static final int TEST_UID2 = 10002;
+ private static final int TEST_UID3 = 10003;
+
+ @Before
+ public void setUp() {
+ // Share classloader to allow package private access.
+ System.setProperty("dexmaker.share_classloader", "true");
+
+ mContext = InstrumentationRegistry.getTargetContext();
+ mHandler = new TestHandler(Looper.getMainLooper());
+ mService = new BlobStoreManagerService(mContext, new TestInjector());
+ mUserSessions = new LongSparseArray<>();
+ mUserBlobs = new ArrayMap<>();
+
+ mService.addUserSessionsForTest(mUserSessions, UserHandle.myUserId());
+ mService.addUserBlobsForTest(mUserBlobs, UserHandle.myUserId());
+
+ mStateChangeListener = mService.new SessionStateChangeListener();
+ }
+
+ @Test
+ public void testHandlePackageRemoved() throws Exception {
+ // Setup sessions
+ final File sessionFile1 = mock(File.class);
+ final long sessionId1 = 11;
+ final BlobStoreSession session1 = createBlobStoreSessionMock(TEST_PKG1, TEST_UID1,
+ sessionId1, sessionFile1);
+ mUserSessions.append(sessionId1, session1);
+
+ final File sessionFile2 = mock(File.class);
+ final long sessionId2 = 25;
+ final BlobStoreSession session2 = createBlobStoreSessionMock(TEST_PKG2, TEST_UID2,
+ sessionId2, sessionFile2);
+ mUserSessions.append(sessionId2, session2);
+
+ final File sessionFile3 = mock(File.class);
+ final long sessionId3 = 37;
+ final BlobStoreSession session3 = createBlobStoreSessionMock(TEST_PKG3, TEST_UID3,
+ sessionId3, sessionFile3);
+ mUserSessions.append(sessionId3, session3);
+
+ final File sessionFile4 = mock(File.class);
+ final long sessionId4 = 48;
+ final BlobStoreSession session4 = createBlobStoreSessionMock(TEST_PKG1, TEST_UID1,
+ sessionId4, sessionFile4);
+ mUserSessions.append(sessionId4, session4);
+
+ // Setup blobs
+ final File blobFile1 = mock(File.class);
+ final BlobHandle blobHandle1 = BlobHandle.createWithSha256("digest1".getBytes(),
+ "label1", System.currentTimeMillis(), "tag1");
+ final BlobMetadata blobMetadata1 = createBlobMetadataMock(blobFile1, true);
+ mUserBlobs.put(blobHandle1, blobMetadata1);
+
+ final File blobFile2 = mock(File.class);
+ final BlobHandle blobHandle2 = BlobHandle.createWithSha256("digest2".getBytes(),
+ "label2", System.currentTimeMillis(), "tag2");
+ final BlobMetadata blobMetadata2 = createBlobMetadataMock(blobFile2, false);
+ mUserBlobs.put(blobHandle2, blobMetadata2);
+
+ // Invoke test method
+ mService.handlePackageRemoved(TEST_PKG1, TEST_UID1);
+
+ // Verify sessions are removed
+ verify(sessionFile1).delete();
+ verify(sessionFile2, never()).delete();
+ verify(sessionFile3, never()).delete();
+ verify(sessionFile4).delete();
+
+ assertThat(mUserSessions.size()).isEqualTo(2);
+ assertThat(mUserSessions.get(sessionId1)).isNull();
+ assertThat(mUserSessions.get(sessionId2)).isNotNull();
+ assertThat(mUserSessions.get(sessionId3)).isNotNull();
+ assertThat(mUserSessions.get(sessionId4)).isNull();
+
+ // Verify blobs are removed
+ verify(blobMetadata1).removeCommitter(TEST_PKG1, TEST_UID1);
+ verify(blobMetadata1).removeLeasee(TEST_PKG1, TEST_UID1);
+ verify(blobMetadata2).removeCommitter(TEST_PKG1, TEST_UID1);
+ verify(blobMetadata2).removeLeasee(TEST_PKG1, TEST_UID1);
+
+ verify(blobFile1, never()).delete();
+ verify(blobFile2).delete();
+
+ assertThat(mUserBlobs.size()).isEqualTo(1);
+ assertThat(mUserBlobs.get(blobHandle1)).isNotNull();
+ assertThat(mUserBlobs.get(blobHandle2)).isNull();
+ }
+
+ private BlobStoreSession createBlobStoreSessionMock(String ownerPackageName, int ownerUid,
+ long sessionId, File sessionFile) {
+ final BlobStoreSession session = mock(BlobStoreSession.class);
+ when(session.getOwnerPackageName()).thenReturn(ownerPackageName);
+ when(session.getOwnerUid()).thenReturn(ownerUid);
+ when(session.getSessionId()).thenReturn(sessionId);
+ when(session.getSessionFile()).thenReturn(sessionFile);
+ return session;
+ }
+
+ private BlobMetadata createBlobMetadataMock(File blobFile, boolean hasLeases) {
+ final BlobMetadata blobMetadata = mock(BlobMetadata.class);
+ when(blobMetadata.getBlobFile()).thenReturn(blobFile);
+ when(blobMetadata.hasLeases()).thenReturn(hasLeases);
+ return blobMetadata;
+ }
+
+ private class TestHandler extends Handler {
+ TestHandler(Looper looper) {
+ super(looper);
+ }
+
+ @Override
+ public void dispatchMessage(Message msg) {
+ // Ignore all messages
+ }
+ }
+
+ private class TestInjector extends Injector {
+ @Override
+ public Handler initializeMessageHandler() {
+ return mHandler;
+ }
+ }
+}