diff --git a/legacy/src/main/java/android/content/res/XResources.java b/legacy/src/main/java/android/content/res/XResources.java index 2a5850ebe4..71852fbcc1 100644 --- a/legacy/src/main/java/android/content/res/XResources.java +++ b/legacy/src/main/java/android/content/res/XResources.java @@ -33,10 +33,10 @@ import org.xmlpull.v1.XmlPullParser; import java.io.File; -import java.util.Arrays; import java.util.HashMap; import java.util.LinkedList; import java.util.WeakHashMap; +import java.util.concurrent.atomic.AtomicIntegerArray; import de.robv.android.xposed.IXposedHookZygoteInit; import de.robv.android.xposed.XC_MethodHook; @@ -66,9 +66,16 @@ public class XResources extends XResourcesSuperClass { // TT is the type of the resource; // NNNN is the name of the resource in that type. // For applications resources, PP is always 0x7f. - private static final byte[] sSystemReplacementsCache = new byte[256]; // bitmask: 0x000700ff => 2048 bit => 256 bytes - private byte[] mReplacementsCache; // bitmask: 0x0007007f => 1024 bit => 128 bytes - private static final HashMap sReplacementsCacheMap = new HashMap<>(); + // + // Which ids are worth a lookup, one bit each, a bit standing for several ids. Ints rather than + // bytes because an AtomicIntegerArray element is read with a volatile load: the thread that + // registers a replacement is rarely the thread that reads the resource, and a plain array read + // is free to miss the other thread's write for as long as it likes, which here means serving + // the original resource for as long as it likes. The per-directory one is final so that the + // array is in place for every thread that can see the object at all. + private static final AtomicIntegerArray sSystemReplacementsCache = new AtomicIntegerArray(64); // 0x000700ff => 2048 bits + private final AtomicIntegerArray mReplacementsCache; // 0x0007007f => 1024 bits + private static final HashMap sReplacementsCacheMap = new HashMap<>(); private static final SparseArray sColorStateListCache = new SparseArray<>(0); private static final SparseArray>> sLayoutCallbacks = new SparseArray<>(); @@ -93,19 +100,20 @@ public XResources(ClassLoader classLoader, String resDir) { this.mResDir = resDir; this.mPackageName = getPackageName(resDir); + AtomicIntegerArray cache = null; if (resDir != null) { synchronized (sReplacementsCacheMap) { // Not computeIfAbsent: its mapping function would be a lambda, which this class may // not create. See the note above [includedLayouts]. Under this lock the two spellings // describe the same operation. - byte[] cache = sReplacementsCacheMap.get(resDir); + cache = sReplacementsCacheMap.get(resDir); if (cache == null) { - cache = new byte[128]; + cache = new AtomicIntegerArray(32); sReplacementsCacheMap.put(resDir, cache); } - mReplacementsCache = cache; } } + mReplacementsCache = cache; } /** @@ -169,7 +177,7 @@ public boolean isFirstLoad() { for (int i = 0; i < sReplacements.size(); i++) { sReplacements.valueAt(i).remove(mResDir); } - Arrays.fill(mReplacementsCache, (byte) 0); + clearCacheBits(mReplacementsCache); return true; } } @@ -330,8 +338,8 @@ private ResourceNames getResourceNames(int id) { return new ResourceNames( id, getResourcePackageName(id), - getResourceTypeName(id), - getResourceEntryName(id)); + getResourceEntryName(id), + getResourceTypeName(id)); } private static ResourceNames getSystemResourceNames(int id) { @@ -339,8 +347,8 @@ private static ResourceNames getSystemResourceNames(int id) { return new ResourceNames( id, sysRes.getResourcePackageName(id), - sysRes.getResourceTypeName(id), - sysRes.getResourceEntryName(id)); + sysRes.getResourceEntryName(id), + sysRes.getResourceTypeName(id)); } private static void putResourceNames(String resDir, ResourceNames resNames) { @@ -605,19 +613,6 @@ else if (resDir == null && id >= 0x7f000000) if (replacement instanceof Drawable) throw new IllegalArgumentException("Drawable replacements are deprecated since Xposed 2.1. Use DrawableLoader instead."); - // Cache that we have a replacement for this ID, false positives are accepted to save memory. - if (id < 0x7f000000) { - int cacheKey = (id & 0x00070000) >> 11 | (id & 0xf8) >> 3; - synchronized (sSystemReplacementsCache) { - sSystemReplacementsCache[cacheKey] |= 1 << (id & 7); - } - } else { - int cacheKey = (id & 0x00070000) >> 12 | (id & 0x78) >> 3; - synchronized (res.mReplacementsCache) { - res.mReplacementsCache[cacheKey] |= 1 << (id & 7); - } - } - synchronized (sReplacements) { HashMap inner = sReplacements.get(id); if (inner == null) { @@ -625,9 +620,53 @@ else if (resDir == null && id >= 0x7f000000) sReplacements.put(id, inner); } inner.put(resDir, replacement); + + // Under the same lock as the row, because isFirstLoad drops a directory's rows and + // clears its bits while holding it: a bit set outside the lock can be cleared while + // the row it stands for is written, and the replacement then answers nothing, for good. + // False positives are accepted to save memory, false negatives are not survivable. + if (id < 0x7f000000) + setCacheBit(sSystemReplacementsCache, systemCacheBit(id)); + else + setCacheBit(res.mReplacementsCache, appCacheBit(id)); + } + } + + /** The cache bit standing for a framework id, that is {@code 0x000700ff} of it. */ + private static int systemCacheBit(int id) { + return (id & 0x00070000) >> 8 | (id & 0xff); + } + + /** The cache bit standing for an app id, that is {@code 0x0007007f} of it. */ + private static int appCacheBit(int id) { + return (id & 0x00070000) >> 9 | (id & 0x7f); + } + + private static boolean hasCacheBit(AtomicIntegerArray cache, int bit) { + return (cache.get(bit >> 5) & (1 << (bit & 31))) != 0; + } + + /** + * Sets [bit], retrying until the word carries it. + * + * A loop rather than getAndUpdate, whose lambda would name this class. See the note above + * [includedLayouts]. + */ + private static void setCacheBit(AtomicIntegerArray cache, int bit) { + int index = bit >> 5; + int mask = 1 << (bit & 31); + for (;;) { + int word = cache.get(index); + if ((word & mask) != 0 || cache.compareAndSet(index, word, word | mask)) + return; } } + private static void clearCacheBits(AtomicIntegerArray cache) { + for (int i = 0; i < cache.length(); i++) + cache.set(i, 0); + } + // ======================================================= // RETURNING REPLACEMENTS // ======================================================= @@ -638,12 +677,10 @@ private Object getReplacement(int id) { // Check the cache whether it's worth looking for replacements if (id < 0x7f000000) { - int cacheKey = (id & 0x00070000) >> 11 | (id & 0xf8) >> 3; - if ((sSystemReplacementsCache[cacheKey] & (1 << (id & 7))) == 0) + if (!hasCacheBit(sSystemReplacementsCache, systemCacheBit(id))) return null; } else if (mResDir != null) { - int cacheKey = (id & 0x00070000) >> 12 | (id & 0x78) >> 3; - if ((mReplacementsCache[cacheKey] & (1 << (id & 7))) == 0) + if (!hasCacheBit(mReplacementsCache, appCacheBit(id))) return null; } @@ -974,7 +1011,10 @@ public XmlResourceParser getLayout(int id) throws NotFoundException { } if (callbacks != null) { String variant = "layout"; - TypedValue value = (TypedValue) getObjectField(this, "mTmpValue"); + // Resources' mTmpValue is a one-slot pool, emptied by obtainTempTypedValue + // until its holder releases it, so a lookup in flight on these Resources + // leaves nothing there to read this through. + TypedValue value = new TypedValue(); getValue(id, value, true); if (value.type == TypedValue.TYPE_STRING) { String[] components = value.string.toString().split("/", 3); @@ -984,7 +1024,7 @@ public XmlResourceParser getLayout(int id) throws NotFoundException { XposedBridge.log("Unexpected resource path \"" + value.string.toString() + "\" for resource id 0x" + Integer.toHexString(id)); } else { - XposedBridge.log(new NotFoundException("Could not find file name for resource id 0x") + Integer.toHexString(id)); + XposedBridge.log(new NotFoundException("Could not find file name for resource id 0x" + Integer.toHexString(id))); } synchronized (sXmlInstanceDetails) { @@ -992,7 +1032,13 @@ public XmlResourceParser getLayout(int id) throws NotFoundException { HashMap resNamesInner = sResourceNames.get(id); if (resNamesInner != null) { synchronized (resNamesInner) { - XMLInstanceDetails details = new XMLInstanceDetails(resNamesInner.get(mResDir), variant, callbacks); + // Both keys, as the callbacks above were looked up under: a + // system-wide hook files its names under null, and the layouts it + // hooks are inflated by apps, whose resource directory is not null. + ResourceNames resNames = resNamesInner.get(mResDir); + if (resNames == null && mResDir != null) + resNames = resNamesInner.get(null); + XMLInstanceDetails details = new XMLInstanceDetails(resNames, variant, callbacks); sXmlInstanceDetails.put(result, details); // if we were called inside LayoutInflater.parseInclude, store the details for it @@ -1803,6 +1849,11 @@ private static XC_LayoutInflated.Unhook hookLayoutInternal(String resDir, int id } } + // Ahead of the callbacks, because an inflation reads the two in the other order: it takes + // the callbacks and then the names, and one that finds callbacks with no names records + // nothing for the document it is about to return, so later callbacks miss it too. + putResourceNames(resDir, resNames); + HashMap> inner; synchronized (sLayoutCallbacks) { inner = sLayoutCallbacks.get(id); @@ -1812,19 +1863,19 @@ private static XC_LayoutInflated.Unhook hookLayoutInternal(String resDir, int id } } - CopyOnWriteSortedSet callbacks; synchronized (inner) { - callbacks = inner.get(resDir); + CopyOnWriteSortedSet callbacks = inner.get(resDir); if (callbacks == null) { + // Filled before it is published: a set that can be found while it is still empty + // is a hook that is registered and does not run. callbacks = new CopyOnWriteSortedSet<>(); + callbacks.add(callback); inner.put(resDir, callbacks); + } else { + callbacks.add(callback); } } - callbacks.add(callback); - - putResourceNames(resDir, resNames); - return callback.new Unhook(resDir, id); }