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