Cache binder LocalCallingIdentity across requests
I9811efca7af223eaf99e28c79a948eca41579dbe introduced
LocalCallingIdentity caching across FUSE requests.
Now we extend (and share) the cache to binder requests.
This allows us store data on the LCI in the short term that can be
accessible by the same uid across binder or FUSE requests.
Specifically, it will allow us save deleted db rows via FUSE while
allowing short term access from the uid via binder.
Test: atest FuseDaemonHostTest
Bug: 148904081
Change-Id: I6be0e8812bb53d5aeff58516d23f1378768fcee1
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 67b7d86..d4a83b6 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -256,6 +256,8 @@
private static final String INCLUDED_DEFAULT_DIRECTORIES =
"android:included-default-directories";
+ private static final int UNKNOWN_UID = -1;
+
/**
* Set of {@link Cursor} columns that refer to raw filesystem paths.
*/
@@ -343,8 +345,8 @@
/**
* Map from UID to cached {@link LocalCallingIdentity}. Values are only
- * maintained in this map while the UID is actively working with a
- * performance-critical component, such as camera.
+ * maintained in this map until there's any change in the appops needed or packages
+ * used in the {@link LocalCallingIdentity}.
*/
@GuardedBy("mCachedCallingIdentity")
private final SparseArray<LocalCallingIdentity> mCachedCallingIdentity = new SparseArray<>();
@@ -362,24 +364,28 @@
}
};
- /**
- * Map from UID to cached {@link LocalCallingIdentity}. Values are only
- * maintained in this map until there's any change in the appops needed or packages
- * used in the {@link LocalCallingIdentity}.
- */
- @GuardedBy("mCachedCallingIdentityForFuse")
- private final SparseArray<LocalCallingIdentity> mCachedCallingIdentityForFuse =
- new SparseArray<>();
-
private OnOpChangedListener mModeListener =
- (op, packageName) -> invalidateLocalCallingIdentityCache(packageName, "op " + op);
+ (op, packageName) -> invalidateLocalCallingIdentityCache(UNKNOWN_UID, packageName,
+ "op " + op);
private LocalCallingIdentity getCachedCallingIdentityForFuse(int uid) {
- synchronized (mCachedCallingIdentityForFuse) {
- LocalCallingIdentity ident = mCachedCallingIdentityForFuse.get(uid);
+ synchronized (mCachedCallingIdentity) {
+ LocalCallingIdentity ident = mCachedCallingIdentity.get(uid);
if (ident == null) {
- ident = LocalCallingIdentity.fromExternal(getContext(), uid);
- mCachedCallingIdentityForFuse.put(uid, ident);
+ ident = LocalCallingIdentity.fromExternal(getContext(), uid);
+ mCachedCallingIdentity.put(uid, ident);
+ }
+ return ident;
+ }
+ }
+
+ private LocalCallingIdentity getCachedCallingIdentityForBinder() {
+ int uid = Binder.getCallingUid();
+ synchronized (mCachedCallingIdentity) {
+ LocalCallingIdentity ident = mCachedCallingIdentity.get(uid);
+ if (ident == null) {
+ ident = LocalCallingIdentity.fromBinder(getContext(), this);
+ mCachedCallingIdentity.put(uid, ident);
}
return ident;
}
@@ -391,14 +397,7 @@
* call is finished.
*/
private final ThreadLocal<LocalCallingIdentity> mCallingIdentity = ThreadLocal
- .withInitial(() -> {
- synchronized (mCachedCallingIdentity) {
- final LocalCallingIdentity cached = mCachedCallingIdentity
- .get(Binder.getCallingUid());
- return (cached != null) ? cached
- : LocalCallingIdentity.fromBinder(getContext(), this);
- }
- });
+ .withInitial(this::getCachedCallingIdentityForBinder);
/**
* We simply propagate the UID that is being tracked by
@@ -480,7 +479,9 @@
Uri uri = intent.getData();
String pkg = uri != null ? uri.getSchemeSpecificPart() : null;
if (pkg != null) {
- invalidateLocalCallingIdentityCache(pkg, "package " + intent.getAction());
+ int uid = intent.getIntExtra(Intent.EXTRA_UID, UNKNOWN_UID);
+ invalidateLocalCallingIdentityCache(uid, pkg,
+ "package " + intent.getAction());
} else {
Log.w(TAG, "Failed to retrieve package from intent: " + intent.getAction());
}
@@ -489,15 +490,21 @@
}
};
- private void invalidateLocalCallingIdentityCache(String packageName, String reason) {
- synchronized (mCachedCallingIdentityForFuse) {
- try {
- Log.i(TAG, "Invalidating LocalCallingIdentity cache for package " + packageName
- + ". Reason: " + reason);
- mCachedCallingIdentityForFuse.remove(
- getContext().getPackageManager().getPackageUid(packageName, 0));
- } catch (NameNotFoundException ignored) {
+ private void invalidateLocalCallingIdentityCache(int uid, String packageName, String reason) {
+ try {
+ if (uid == UNKNOWN_UID) {
+ uid = getContext().getPackageManager().getPackageUid(packageName, 0);
}
+ } catch (NameNotFoundException e) {
+ Log.wtf(TAG, "Failed to invalidate LocalCallingIdentity cache for package: "
+ + packageName, e);
+ return;
+ }
+
+ synchronized (mCachedCallingIdentity) {
+ Log.i(TAG, "Invalidating LocalCallingIdentity cache for package " + packageName
+ + ". Uid: " + uid + ". Reason: " + reason);
+ mCachedCallingIdentity.remove(uid);
}
}
@@ -816,7 +823,6 @@
AppOpsManager.OPSTR_CAMERA
}, context.getMainExecutor(), mActiveListener);
-
mAppOpsManager.startWatchingMode(AppOpsManager.OPSTR_READ_EXTERNAL_STORAGE,
null /* all packages */, mModeListener);
mAppOpsManager.startWatchingMode(AppOpsManager.OPSTR_WRITE_EXTERNAL_STORAGE,
@@ -962,6 +968,11 @@
mDirectoryCache.clear();
}
+ // Purge any per uid caches
+ synchronized (mCachedCallingIdentity) {
+ mCachedCallingIdentity.clear();
+ }
+
final long durationMillis = (SystemClock.elapsedRealtime() - startTime);
Metrics.logIdleMaintenance(MediaStore.VOLUME_EXTERNAL, helper.getItemCount(),
durationMillis, staleThumbnails, expiredMedia);
diff --git a/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java b/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java
index f7af146..d13886a 100644
--- a/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java
+++ b/tests/jni/FuseDaemonTest/FilePathAccessTestHelper/src/com/android/tests/fused/FilePathAccessTestHelper.java
@@ -25,7 +25,9 @@
import static com.android.tests.fused.lib.TestUtils.OPEN_FILE_FOR_READ_QUERY;
import static com.android.tests.fused.lib.TestUtils.OPEN_FILE_FOR_WRITE_QUERY;
import static com.android.tests.fused.lib.TestUtils.QUERY_TYPE;
+import static com.android.tests.fused.lib.TestUtils.canOpenWithMediaProvider;
import static com.android.tests.fused.lib.TestUtils.canOpen;
+import static com.android.tests.fused.lib.TestUtils.setContext;
import android.app.Activity;
import android.content.Intent;
@@ -49,6 +51,7 @@
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
+ setContext(this);
String queryType = getIntent().getStringExtra(QUERY_TYPE);
queryType = queryType == null ? "null" : queryType;
switch (queryType) {
@@ -115,9 +118,11 @@
} else if (queryType.equals(DELETE_FILE_QUERY)) {
returnStatus = file.delete();
} else if (queryType.equals(OPEN_FILE_FOR_READ_QUERY)) {
- returnStatus = canOpen(file, false /* forWrite */);
+ returnStatus = canOpen(file, /* forWrite */ false)
+ && canOpenWithMediaProvider(file, /* forWrite */ false);
} else if (queryType.equals(OPEN_FILE_FOR_WRITE_QUERY)) {
- returnStatus = canOpen(file, true /* forWrite */);
+ returnStatus = canOpen(file, /* forWrite */ true)
+ && canOpenWithMediaProvider(file, /* forWrite */ true);
}
} catch(IOException e) {
Log.e(TAG, "Failed to access file: " + filePath + ". Query type: " + queryType, e);
diff --git a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
index 97e39d4..2bbb4b3 100644
--- a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
+++ b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
@@ -93,6 +93,8 @@
private static final long POLLING_TIMEOUT_MILLIS = TimeUnit.SECONDS.toMillis(10);
private static final long POLLING_SLEEP_MILLIS = 100;
+ @Nullable private static Context sContext;
+
/**
* Grants {@link Manifest.permission#GRANT_RUNTIME_PERMISSIONS} to the given package.
*/
@@ -255,9 +257,16 @@
}
public static ContentResolver getContentResolver() {
+ if (sContext != null) {
+ return sContext.getContentResolver();
+ }
return getContext().getContentResolver();
}
+ public static void setContext(Context context) {
+ sContext = context;
+ }
+
/**
* Queries {@link ContentResolver} for a file and returns the corresponding {@link Uri} for its
* entry in the database. Returns {@code null} if file doesn't exist in the database.
@@ -381,7 +390,6 @@
assertThat(fileUri).isNotNull();
Log.i(TAG, "Uri: " + fileUri + ". Data: " + file.getPath());
ParcelFileDescriptor pfd = getContentResolver().openFileDescriptor(fileUri, mode);
- assertThat(pfd).isNotNull();
return pfd;
}
@@ -486,6 +494,19 @@
}
}
+ public static boolean canOpenWithMediaProvider(File file, boolean forWrite) {
+ try {
+ final Uri fileUri = getFileUri(file);
+ if (fileUri == null) {
+ return false;
+ }
+ getContentResolver().openFileDescriptor(fileUri, forWrite ? "w" : "r");
+ return true;
+ } catch (Exception e) {
+ return false;
+ }
+ }
+
public static void pollForExternalStorageState() throws Exception {
for (int i = 0; i < POLLING_TIMEOUT_MILLIS / POLLING_SLEEP_MILLIS; i++) {
if(Environment.getExternalStorageState(Environment.getExternalStorageDirectory())