diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt index e0e2280b4..24bdf1f8b 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt @@ -205,6 +205,27 @@ object VectorService : IVectorDaemon.Stub() { override fun onUidIdle(uid: Int, disabled: Boolean) = ModuleAppService.uidStarts(uid) override fun onUidGone(uid: Int, disabled: Boolean) = ModuleAppService.uidGone(uid) + + // Not registered for, and so never dispatched: the platform gates these on + // UID_OBSERVER_PROCSTATE, UID_OBSERVER_CAPABILITY and UID_OBSERVER_PROC_OOM_ADJ, and the + // mask below asks for none of them. They are overridden because the framework interface + // declares them and a Stub subclass that leaves one abstract dies on the first call -- + // the callback is oneway, so the resulting Error is fatal to this process rather than + // returned to anyone. Both shapes of each are here because both exist in the range this + // supports: onUidStateChanged gained its capability in 30, and onUidProcAdjChanged + // arrived in 33 and gained its adj in 34. + override fun onUidStateChanged(uid: Int, procState: Int, procStateSeq: Long) {} + + override fun onUidStateChanged( + uid: Int, + procState: Int, + procStateSeq: Long, + capability: Int + ) {} + + override fun onUidProcAdjChanged(uid: Int) {} + + override fun onUidProcAdjChanged(uid: Int, adj: Int) {} } val which = diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleAppService.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleAppService.kt index bdcbb8339..916ac2cb4 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleAppService.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleAppService.kt @@ -4,8 +4,10 @@ import android.content.AttributionSource import android.os.Binder import android.os.Build import android.os.Bundle +import android.os.IBinder import android.os.ParcelFileDescriptor import android.os.RemoteException +import android.os.SystemClock import android.util.Log import io.github.libxposed.service.HookedProcess import io.github.libxposed.service.IHotReloadCallback @@ -57,26 +59,184 @@ class ModuleAppService(private val loadedModule: LoadedModule) : IXposedService. // later request with IN_PROGRESS for as long as the process lives. private const val RELOAD_TIMEOUT_SECONDS = 30L + /** + * The uids whose module app is holding a binder we handed it. + * + * A binder belongs to the *process* that received it, but a uid can outlive any one of its + * processes: an app with a `:remote` or crash-handler process, or a shared user id, keeps its + * uid alive when the process we served is reaped, so no [uidGone] arrives and the replacement + * process would be refused here forever. That was unreachable while the reference below pinned + * every module app at foreground priority and nothing ever reaped it. Giving the reference back + * makes it the ordinary case, so entries are also dropped by [linkDelivery] when the process + * that took the binder dies. + * + * Recorded on a *successful* send rather than on the attempt: a failed send leaves nothing on + * the other side, and treating it as delivered meant the one module that most needed another + * attempt never got one. + */ private val uidSet = ConcurrentHashMap.newKeySet() + + /** The uids a send is running for right now, so the three observer callbacks agree on one. */ + private val sending = ConcurrentHashMap.newKeySet() + + /** + * What tells [uidSet] that a delivery is over: the provider binder we spoke to, and the + * recipient watching it. Held because a `DeathRecipient` nothing references is one the runtime + * may collect before it ever fires. + */ + private val deliveries = ConcurrentHashMap>() + private val serviceMap = Collections.synchronizedMap(WeakHashMap()) + /** + * Consecutive failed sends per uid, and when the last one was. + * + * A module app that dies before it can publish its provider is not a transient failure to be + * retried at the speed of the uid observer. It happens — an app that crashes on start, or one + * another module deliberately kills, as in #889 where a module in a third module's scope took + * its host down on every launch — and the delivery below *starts the process*, so retrying is + * not a passive act: it feeds the very loop it is failing on. Fourteen starts in seventy-six + * seconds were observed that way, six of them ours. + * + * Per uid and not per package, because `getModuleByUid` matches on the app id: one module + * installed for two users is one `LoadedModule` under two uids, and keying by name would let a + * crash-looping copy in a work profile throttle the healthy copy in user 0, and let either + * one's success wipe the other's run. + * + * Once [MAX_CONSECUTIVE_BINDER_FAILURES] have piled up the retries are throttled to one per + * [BINDER_RETRY_COOLDOWN_MS] — the count is held at the ceiling rather than reset by the + * attempt that the cooldown lets through, or the ceiling would simply be re-climbed and three + * more attempts allowed every minute for ever. A run is forgotten after + * [BINDER_FAILURE_RUN_MS] without a failure, so an occasional one never accumulates. Throttled + * rather than abandoned, and cleared by the first success, because the app may simply have been + * mid-update or out of memory; a module written off for good on three failures would be a worse + * bug than the one this is fixing. + */ + private val binderFailures = ConcurrentHashMap() + + private class FailureRun(val count: Int, val atElapsed: Long) + + private const val MAX_CONSECUTIVE_BINDER_FAILURES = 3 + private const val BINDER_RETRY_COOLDOWN_MS = 60_000L + private const val BINDER_FAILURE_RUN_MS = 10 * BINDER_RETRY_COOLDOWN_MS + + // The delivery blocks in getContentProviderExternal until the app publishes its provider or + // AMS gives up on it, and it runs from an IUidObserver callback - one binder thread, serving + // every uid transition on the device. A module app that never publishes therefore stalls the + // delivery of every *other* module's binder behind it: eight and a half seconds, measured, on + // a device where one module app was crash-looping. One thread per module keeps that local. + private val binderExecutor = + Executors.newCachedThreadPool { r -> Thread(r, "vector-module-binder") } + fun uidClear() { uidSet.clear() } fun uidStarts(uid: Int) { - if (uidSet.add(uid)) { - val module = ConfigCache.getModuleByUid(uid) - if (module?.code?.legacy == false) { - val service = serviceMap.getOrPut(module) { ModuleAppService(module) } - service.sendBinder(uid) - } + if (uid in uidSet || !sending.add(uid)) return + val module = ConfigCache.getModuleByUid(uid) + if (module?.code?.legacy != false) { + sending.remove(uid) + return + } + if (isThrottled(uid)) { + sending.remove(uid) + return + } + val service = serviceMap.getOrPut(module) { ModuleAppService(module) } + // Off the observer thread, and never inline: see [binderExecutor]. Caught, because a uid + // left in [sending] by a rejected submission is one this never looks at again. + runCatching { + binderExecutor.execute { + try { + val delivered = service.sendBinder(uid) + if (delivered != null) { + uidSet.add(uid) + binderFailures.remove(uid) + linkDelivery(uid, delivered) + } else { + recordFailure(uid, module.packageName) + } + } finally { + sending.remove(uid) + } + } + } + .onFailure { + sending.remove(uid) + Log.w(TAG, "Could not schedule the binder delivery for ${module.packageName}", it) + } + } + + /** + * Watches the process that took the binder, so [uidSet] forgets the uid when it dies. + * + * [uidGone] is not enough on its own — it only fires when the *uid* has no processes left — + * and this is what makes a second delivery to a restarted module app possible. A death + * recipient on a proxy is not a client of anything, so unlike the provider reference it puts + * no floor under the process's priority. + */ + private fun linkDelivery(uid: Int, provider: IBinder) { + val recipient = IBinder.DeathRecipient { uidSet.remove(uid) } + runCatching { + provider.linkToDeath(recipient, 0) + deliveries.put(uid, provider to recipient)?.let { (old, previous) -> + runCatching { old.unlinkToDeath(previous, 0) } + } + } + // Already dead, which is an answer in itself: whatever took the binder is gone, so the + // uid must not stay marked as served. + .onFailure { uidSet.remove(uid) } + } + + /** True while a uid has spent its attempts and its cooldown has not elapsed. */ + private fun isThrottled(uid: Int): Boolean { + val run = binderFailures[uid] ?: return false + if (run.count < MAX_CONSECUTIVE_BINDER_FAILURES) return false + return SystemClock.elapsedRealtime() - run.atElapsed < BINDER_RETRY_COOLDOWN_MS + } + + private fun recordFailure(uid: Int, modulePkg: String) { + var crossed = false + // Read-modify-write in one step. Two threads cannot be here for one uid while [sending] + // holds, but that is an invariant of another field and not one to build arithmetic on. + binderFailures.compute(uid) { _, previous -> + val now = SystemClock.elapsedRealtime() + val count = + when { + // A run is forgotten only after a long quiet spell, not after one cooldown. Forgetting + // it at the cooldown meant the attempt the cooldown let through reset the count, so + // the ceiling was re-climbed and three more attempts allowed every minute, for ever. + previous == null || now - previous.atElapsed >= BINDER_FAILURE_RUN_MS -> 1 + // Held at the ceiling rather than growing without bound: what the number decides is + // only whether we are throttled, and pinning it here is what makes the cooldown mean + // one attempt rather than another three. + else -> minOf(previous.count + 1, MAX_CONSECUTIVE_BINDER_FAILURES) + } + crossed = count == MAX_CONSECUTIVE_BINDER_FAILURES && (previous?.count ?: 0) < count + FailureRun(count, now) + } + // Once, on the way past the ceiling. The failures themselves are already logged one by one + // in sendBinder; what is worth saying here is that we have stopped trying, which is the part + // a reader chasing a module that never receives its service cannot otherwise see. + if (crossed) { + Log.w( + TAG, + "$modulePkg/$uid failed to take its binder $MAX_CONSECUTIVE_BINDER_FAILURES times in" + + " a row; retrying at most once every ${BINDER_RETRY_COOLDOWN_MS / 1000}s") } } fun uidGone(uid: Int) { uidSet.remove(uid) + // A send that never returns — `provider.call` runs the module's own onServiceBind, with no + // deadline — would otherwise leave the uid here for the life of the daemon, and every later + // delivery for it refused at the top of uidStarts. + sending.remove(uid) + deliveries.remove(uid)?.let { (binder, recipient) -> + runCatching { binder.unlinkToDeath(recipient, 0) } + } } // Drives the same cycle as a service request, so onHotReloading can still refuse it. @@ -95,18 +255,59 @@ class ModuleAppService(private val loadedModule: LoadedModule) : IXposedService. /** * Forges a ContentProvider call to force the module's target app process to receive this Binder * IPC endpoint without standard Context.bindService() limits. + * + * Called only from [uidStarts], on [binderExecutor] rather than on the uid observer, because + * `getContentProviderExternal` blocks until the app publishes its provider or the platform gives + * up waiting for it. + * + * @return the provider binder of the process that took it, or null if nobody did. The caller + * counts the failures — nothing else distinguishes "the app has its service" from "we asked and + * nobody answered", and conflating the two is what let a module app that dies on every launch + * be started again a second later, for as long as it kept dying — and watches the binder, so + * that the process dying is what makes the next one eligible. */ - private fun sendBinder(uid: Int) { + private fun sendBinder(uid: Int): IBinder? { val name = loadedModule.packageName - runCatching { - val userId = uid / PER_USER_RANGE - val authority = name + AUTHORITY_SUFFIX + val userId = uid / PER_USER_RANGE + val authority = name + AUTHORITY_SUFFIX + // Identifies our reference to the provider so it can be given back, which it never was. That + // reference counts as a live client of the provider — `ContentProviderRecord`'s + // `hasConnectionOrHandle` is `!connections.isEmpty() || hasExternalProcessHandles()`, and the + // second half counts external references with and without a token — and the platform draws two + // conclusions from a live client. + // + // The host is pinned. `OomAdjuster.computeOomAdjLSP` raises a process publishing such a + // provider to FOREGROUND_APP_ADJ and PROCESS_STATE_IMPORTANT_FOREGROUND, recorded as + // `adjType=ext-provider`. So this held every module app on the device at foreground priority + // for as long as it lived, never cached and never trimmed — and a uid kept out of the + // background is a uid that keeps being reported active, which is what wakes the delivery again. + // + // And the host is restarted. A process that dies while a provider of its is still launching is + // restarted for it, `MAX_RETRY_COUNT` = 3 times per provider record, after which the record is + // dropped and the platform gives up. Taking the reference again builds a fresh record with the + // count back at zero, so re-acquiring on every uid callback is what turned the platform's + // bounded retry into an unbounded one — see the throttle in [binderFailures]. + // + // A real token rather than null also buys a death link: the platform builds a handle object + // around it and releases the reference itself if we die. A null token is only a counter, with + // nothing to link, which is why the old leak was permanent rather than merely long. + val token = Binder() + return runCatching { + // The tag argument arrived in Q, replacing the three-argument form rather than + // overloading it, so each side of that line is a NoSuchMethodError on the other. The + // branch was in the Java daemon and was lost in the Kotlin rewrite (#597), which means + // no modern module has been handed its service on 8.1 or 9 since — swallowed, because + // the error lands in the runCatching below and reads as an ordinary failed delivery. val provider = - activityManager?.getContentProviderExternal(authority, userId, null, null)?.provider + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + activityManager?.getContentProviderExternal(authority, userId, token, "vector") + } else { + activityManager?.getContentProviderExternal(authority, userId, token) + }?.provider if (provider == null) { Log.d(TAG, "No service provider for $name") - return + return@runCatching null } val extra = Bundle().apply { putBinder("binder", asBinder()) } @@ -126,10 +327,54 @@ class ModuleAppService(private val loadedModule: LoadedModule) : IXposedService. provider.call("android", SEND_BINDER, null, extra) } - if (reply != null) Log.d(TAG, "Sent module binder to $name") - else Log.w(TAG, "Failed to send module binder to $name") + if (reply != null) { + Log.d(TAG, "Sent module binder to $name") + provider.asBinder() + } else { + Log.w(TAG, "Failed to send module binder to $name") + null + } } .onFailure { Log.w(TAG, "Failed to send module binder for uid $uid", it) } + // Unconditionally, and not only when a provider came back. The platform registers the + // external client *before* it waits for the app to publish, and the two returns that + // matter here — the app died while launching, and the wait timed out — come after that + // registration with the reference still held. Those are exactly the returns a module app + // that dies on every start produces, so releasing only on success would have left the + // restart loop this method exists to stop completely intact. + // + // Asking when nothing was registered is not free of consequence, only of harm. If the app + // is not running there is no record and the platform returns quietly; if it is, the record + // exists under this authority whether or not our acquire got as far as registering, and + // the platform logs that something tried to remove an external reference it does not have. + // A line in its log against the loop this stops is the right side of that trade. + .also { releaseProvider(authority, token, userId) } + .getOrNull() + } + + /** + * Gives back the reference [sendBinder] took, whatever became of the call in between. + * + * The user id has to be named, and can be from Q. The plain form is all that API 27 and 28 have, + * and there the platform resolves the name against the *caller's* user, which is the daemon's: + * a reference taken for a module in a secondary user cannot be given back at all, and asking + * anyway would decrement the token-less counter of whatever record user 0 has under that name. + * So on those two releases a secondary user's reference is left to the token instead — the + * platform links the handle to the token's death, so the reference goes when the daemon does. + */ + private fun releaseProvider(authority: String, token: Binder, userId: Int) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q && userId != 0) { + Log.d(TAG, "Cannot release the reference for $authority in user $userId before Q") + return + } + runCatching { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + activityManager?.removeContentProviderExternalAsUser(authority, token, userId) + } else { + activityManager?.removeContentProviderExternal(authority, token) + } + } + .onFailure { Log.w(TAG, "Failed to release the provider reference for $authority", it) } } private fun ensureModule(): Int { diff --git a/hiddenapi/stubs/src/main/java/android/app/IActivityManager.java b/hiddenapi/stubs/src/main/java/android/app/IActivityManager.java index e64857b7a..62a8e511a 100644 --- a/hiddenapi/stubs/src/main/java/android/app/IActivityManager.java +++ b/hiddenapi/stubs/src/main/java/android/app/IActivityManager.java @@ -90,6 +90,11 @@ int bindService(IApplicationThread caller, IBinder token, Intent service, ContentProviderHolder getContentProviderExternal(String name, int userId, IBinder token) throws RemoteException; + void removeContentProviderExternal(String name, IBinder token) throws RemoteException; + + @RequiresApi(29) + void removeContentProviderExternalAsUser(String name, IBinder token, int userId) throws RemoteException; + void registerUidObserver(IUidObserver observer, int which, int cutpoint, String callingPackage) throws RemoteException; abstract class Stub extends Binder implements IActivityManager { diff --git a/hiddenapi/stubs/src/main/java/android/app/IUidObserver.java b/hiddenapi/stubs/src/main/java/android/app/IUidObserver.java index 676e509a6..38af361bd 100644 --- a/hiddenapi/stubs/src/main/java/android/app/IUidObserver.java +++ b/hiddenapi/stubs/src/main/java/android/app/IUidObserver.java @@ -2,14 +2,49 @@ import android.os.Binder; +/** + * The union of every method this interface has carried across API 27 to 37, rather than the four + * the daemon happens to listen for. + * + * The real framework class is what a `Stub` subclass extends at runtime, so a method left out here + * is one the subclass cannot override and the platform still finds abstract. The interface is + * `oneway`, so AMS never waits and never sees the failure; an `AbstractMethodError` escaping the + * callback is routed by `JavaBBinder::onTransact` to `report_java_lang_error`, which is fatal, and + * the daemon's own uncaught handler would end the process anyway. That is a loud death for a + * callback nobody asked for. + * + * Nothing dispatches the two absent from the old file today: `onUidStateChanged` and + * `onUidProcAdjChanged` are gated on UID_OBSERVER_PROCSTATE, UID_OBSERVER_CAPABILITY and + * UID_OBSERVER_PROC_OOM_ADJ, and `VectorService` registers for ACTIVE, GONE, IDLE and CACHED. So + * this is insurance against a wider mask -- ours or an OEM's -- not a live crash. + * + * Both signatures are declared for the two methods that changed shape. Only one of each pair + * exists in any given release, and the other is then an ordinary unused method on the subclass. + */ public interface IUidObserver { + /** API 27..37. */ void onUidGone(int uid, boolean disabled); + /** API 27..37. */ void onUidActive(int uid); + /** API 27..37. */ void onUidIdle(int uid, boolean disabled); + /** API 27..29; replaced by the capability overload in 30. */ + void onUidStateChanged(int uid, int procState, long procStateSeq); + + /** API 30..37. */ + void onUidStateChanged(int uid, int procState, long procStateSeq, int capability); + + /** API 33 only; replaced by the adj overload in 34. */ + void onUidProcAdjChanged(int uid); + + /** API 34..37. */ + void onUidProcAdjChanged(int uid, int adj); + + /** API 27..37. */ void onUidCachedChanged(int uid, boolean cached); abstract class Stub extends Binder implements IUidObserver {