Merge "Cache binder LocalCallingIdentity across requests" into rvc-dev
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 79998b3..eb3dd26 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -259,6 +259,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.
*/
@@ -346,8 +348,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<>();
@@ -365,24 +367,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;
}
@@ -394,14 +400,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
@@ -483,7 +482,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());
}
@@ -492,15 +493,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);
}
}
@@ -819,7 +826,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,
@@ -965,6 +971,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())