Move the manager AIDL out of org.lsposed.lspd - #861
Merged
Conversation
…t does The daemon-side AIDL was cleaned in 34144a8, which left this for later: the manager's interface, the parcelables it carries, and the -keep rule naming them. The names had drifted from the code by more than a namespace. getXposedVersionCode returned this framework's version, not Xposed's; getFrameworkCommit had to open its own documentation by saying it was not a commit; enableStatusNotification read as a command and the socket CLI called it as one, from the branch that *reads* settings; dex2oatFlagsLoaded asked whether a property carried a flag; startActivityAsUserWithFeature was named after an AOSP method whose distinguishing feature this signature does not have. Application and UserInfo both collided with platform types, so every caller handling a list of either wrote the type out fully qualified and then kept a local mirror under a name that did say what it was. Three methods had no callers at all - restartFor was a daemon-side no-op with a manager wrapper documenting it as vestigial - and two pairs asked one question twice. getUnloadableModules plus getModuleLoadState is the pair that mattered: the daemon holds a Map<String, Int> and the AIDL sent its keys, then took one transaction per key for the values. That forced the caller to seed every entry with MODULE_LOAD_NO_APK before the second round could overwrite it, so a dropped transaction told the user the framework could not find their module's APK - a claim nothing had established. It is one call returning the map. The lsp_no_switch_to_user intent extra is gone, and it is worth naming because it was a string agreed between two APKs that can ship apart. Getting it wrong switched the device's user and locked the screen when somebody opened a module. It is a parameter. The hand-written transaction ids go with it. They existed to keep a method's number stable across revisions, and they did that badly: id 33 of the old interface carried setHiddenIcon(boolean hide) and later setForcedLauncherIcons(boolean force), the same number with the argument's sense inverted, so every peer built against the earlier file went on calling it and asking for the opposite of what it meant. A number beside each method cannot catch that; a version can. getProtocolVersion is declared first, which makes it transaction zero in every revision, and the manager asks it before anything else. The rule is no longer "append only" but "change what the design wants, and bump PROTOCOL_VERSION in the same commit". That descriptor change is the cost. The manager can be installed as an ordinary app - getManagerApk exists for that - and an installed copy survives a later flash, so a manager and a daemon of different builds is a supported configuration rather than an accident. It used to degrade quietly, one method at a time, which is why ROOT_UNKNOWN has to be 0 and why getLogParts is defended with orEmpty(). A changed descriptor does not degrade: every transaction throws SecurityException while the binder stays alive, so a manager older than the framework it is flashed beside answers nothing on every screen. getProtocolVersion is added at id 1 for the same reason, and can only be added now: every daemon answering to this descriptor was built from a file that already carries it, so it needs no bootstrap sentinel. That is the usual reason a version handshake cannot be retrofitted to a live interface. Every method, constant, field and parcelable now carries what it is for, who may call it, what a false or a null means, and the failure symptom for constraints that cannot be inferred from the signature. Nothing asserts behaviour that was not read out of the implementation first.
The last of this project's own code under org.lsposed.lspd, and the third item 34144a8 left for later. Twenty-two files reach it, across the legacy API, the injected framework and the zygisk bridge, all compiling into one artifact — and nothing in the manager, which wrote its own logger rather than depend on this one. org.matrix.vector.util, deliberately not one of the three prefixes daemon/src/main/jni/obfuscation.cpp rewrites. Nothing here is obfuscated today and this keeps it that way; landing it under .core, .nativebridge or .service would have started rewriting a class the manager does not compile, for no gain. Log becomes a top-level class rather than a member of Utils. It held nothing but static members while being a *non-static inner class*, which Java has accepted only since release 16, and which meant every call site outside an import wrote Utils.Log for a type never scoped to an instance of anything. Splitting it also lets the two say what they are for: Log is the muting-aware stand-in for android.util.Log, and Utils is the convenience layer that logs under the framework's own tag. Which methods honour `muted` is uneven — the message-only forms and w(String, String, Throwable) do, the other Throwable forms do not — and that is carried over exactly. It looks like an oversight, but changing it changes what a muted device records, which is a decision about the log rather than about this move. The tag is the one thing here that must not change: the daemon's log reader routes any tag beginning "Vector" into its verbose stream, so what these helpers write reaches the manager's Verbose tab and travels in an exported bug report. It is now documented where it is declared. XposedBridge keeps its call fully qualified. That file imports android.util.Log too, and it is the framework's own that is wanted there — the platform's getStackTraceString returns an empty string when anything in the cause chain is an UnknownHostException, so a module logging a failed request landed an empty line.
The fourth item 34144a8 left for later. Renaming the AIDL moved the interfaces to honest names and left the classes implementing them behind. ApplicationService implemented IFrameworkService and named neither: nothing in it is about an Application. It is FrameworkService. The two module services were worse than mismatched, they were swapped in the reader's head. A module gets two services — libxposed's IXposedService, which is the module as its own *app* sees it, and this project's IModuleService, which is the module as an injected *process* sees it — and they deliberately differ in what they permit: an app may write its remote files, a hooked process may only read them, because a hooked process runs as the app it was injected into rather than as the module. The class implementing IXposedService was called ModuleService, the vaguer of the two names, and the class implementing IModuleService was called InjectedModuleService. The one name that said which of the two it was sat on the wrong one, and the natural name for the other was already taken. ModuleAppService and InjectedModuleService now both carry the distinguishing fact, and each points at the other. That beats dropping the I from each interface, which would give ModuleService and XposedService — two names told apart only by knowing which project each interface belongs to. The daemon AIDL gets the three fixes that reading it for this turned up: - IVectorDaemon.attachProcess returns null in four cases and documented one of them. The others are a caller that is not uid 1000, a (uid, pid) that has already attached, and a registration that failed. @nullable is documentation-only in the Java backend — no annotation is emitted and the signature is unchanged — so this is the doc catching up with the code, not a wire change. - LoadedModule.applicationInfo was the one field in that file with no comment. It is the *module's* ApplicationInfo, not the host process's, which is why it has to be carried rather than looked up. - IVectorDaemon and IModuleService said nothing about transaction numbering while IFrameworkService explains it at length. Both are implicit and append-only for the same reason — they only ever cross between a daemon and a framework dex from one zip — and both now say so, and why IManagerService is explicit instead.
Eight files carried the GPL boilerplate at the top — the "this program is free software, distributed without warranty, you should have received a copy" paragraph — inherited from upstream and by then naming a project this is not. The licence is conveyed where it counts and is not weakened by this: LICENSE holds the full GPL-3.0 text at the root, and README's own License section says the project is under it. Per-file headers are the FSF's recommendation for making the terms travel with a stray copy of a single file, not a condition of the licence, and twenty lines of boilerplate above a four-line hidden-API stub was never buying that. What the headers held that nothing else did is EdXposed, named in a copyright line on the logger and nowhere else in the repository. It goes to README's Credits beside LSPosed, which already stood there as upstream source — the lineage is a fact about where this code came from and belongs somewhere a reader will find it.
Not part of the rename; it is what the rename makes necessary, and it can be dropped on its own if it is not wanted. The interface's fully qualified name is its binder descriptor, so moving the AIDL out of org.lsposed.lspd changes it. Two of the three consumers ship in the same zip as the daemon and are always in step. The third is not: getManagerApk exists so the manager can be installed as an ordinary app, and an installed copy survives every later flash. Across that skew nothing is loud. Stub.asInterface wraps any binder in a proxy without checking, the binder stays alive so isBinderAlive() keeps answering true, and every transaction throws SecurityException out of the daemon's enforceInterface — which DaemonClient turns into a failed Result and every screen draws as empty. The header would then read "Not activated", which is the one thing that is certainly false: a framework is plainly running, it pushed us the binder, and the reader is being sent to install something they already have. So Constants.setBinder asks the binder what it is before binding. That question is exempt by construction — INTERFACE_TRANSACTION sits outside the range the generated dispatcher checks the interface token for — so it is answered across any mismatch. It can still throw, since the daemon may have died in between, and a throw is not evidence of a mismatch, so it falls through to binding and lets linkToDeath report the death. FrameworkState gains Mismatched, rendered as "Version mismatch". It keeps the error colours — this is a broken install — but takes the priority glyph rather than the cross, because the cross means absence. The label does not blame either side: the descriptor says which build is which, and that goes to the log, but a user cannot act on the difference and the manager is as likely to be the newer one. The two places asking `state != Inactive` to mean "there is a daemon to talk to" would both have answered wrongly, so the question moved onto FrameworkStatus as daemonUsable. HomeViewModel collects the binder and the mismatch together, because a refusal leaves service exactly as it was — null — so a StateFlow has nothing to emit and the header would sit on whatever it already said. There is no route out from inside a stale manager: the call that would fetch a replacement APK is a transaction on the interface that does not match. The parasitic manager ships with the daemon, is always in step, and is reachable from the status notification and the dialer code. A manager built before this check exists cannot show it at all, and every released build is one — on the flash that lands this, such a copy shows empty screens, as it does today for any other daemon failure.
`enableModule` has two paths. Updating an existing row checks what it did — `changed = db.update(...) > 0` — and inserting a newly discovered module did not: it called `db.insert(...)`, threw the result away, and set `changed = true`. `SQLiteDatabase.insert` does not throw on failure. It catches the SQLException itself, logs one line, and answers -1. A disk that is full, a database locked by a concurrent write, or a constraint that rejects the row therefore all looked exactly like a successful insert. Everything downstream then acted on that. `setModuleEnabled` answered true, so the manager left its switch on. `ConfigCache.requestCacheUpdate()` ran for a module that has no row. Worst of the three, the shade's "this module is not activated yet" notice was cancelled — the one thing that would have told the user something was wrong. The module then simply never loaded, with the manager showing it enabled and nothing anywhere reporting a failure.
`systemServerRequested` was set immediately after the uid/process-name/life-token gate, before `FrameworkService.registerHeartBeat` — the call that decides whether system_server actually gets its framework service. Registration answers false when the life token cannot be linked to death, which is rare rather than impossible, and the flag is never cleared. So a failed registration read as attached, and the manager's status page reported the framework as present in system_server while no module hooking the system loaded. That is the expensive kind of wrong: a green health row sends the reader to look at their module, which is the one place the fault is not. The assignment moves below the registration, so the flag now means what `isSystemServerAttached()` says it means. The observable change is that the health row flips a moment later during boot — after the registration rather than at the request — and stays false on the failure it used to hide.
`uninstallPackage` runs on a binder thread, asks the package installer to remove a package, and then waited on a CountDownLatch with no timeout for the status broadcast. The status is not guaranteed to arrive. A device-policy refusal, a user removed while the uninstall is in flight, or a wedged package installer all end with nobody counting the latch down. That thread is then held for the life of the daemon. The cost is not confined to the caller. The daemon's binder thread pool is small, and threads lost this way never come back, so enough of them starve everything else the daemon answers — the manager's module list, the log reads, and the calls injected processes make while they start. The symptom is a manager that hangs on every screen and a device that needs the daemon restarted, none of which points at an uninstall somebody tried a while ago. A minute, which is generous for work that normally takes seconds, and false on expiry: a timeout is a failure of our knowledge rather than of the uninstall, and the caller must not be told a package is gone on the strength of a status that never came.
`muted` gated the message-only forms of d/v/i and both forms of w, but not d/v/i(String, String, Throwable). The setting therefore only half worked: a user who turned verbose logging off still got every debug, verbose and info line that happened to carry an exception. Those are not the rare ones. `muted` is set in injected processes, from `IFrameworkService.isLogMuted`, and the lines that carry a throwable are the ones on the failure paths of hooking — the paths that run in every process the framework touches. A muted device kept paying for them, and kept writing them to a log the user had asked to be quiet. ERROR and above stay ungated, deliberately and as before: muting asks for less noise, not for a failure to go unrecorded.
The daemon only starts the dex2oat wrapper machinery from Android 10 and answers a literal 0 below it — and 0 is DEX2OAT_OK. The status page rendered that as "Supported", in green, with a healthy tick. On Android 8.1 and 9 that is a positive claim about a feature the device does not have. The platform's dex2oat inlines across methods there exactly as it always did, so a hook on an inlined method does not take, and a reader chasing that is told by the status page that this part is fine. The row is dropped below Android 10 rather than given a fifth state. A new constant was the other option and it renders worse: every value the screen does not recognise falls to "Unsupported", so a device that today reads a wrong green would have read a wrong red, which is further from the truth rather than closer. Saying nothing about a wrapper that does not exist is the honest answer, and the section still carries SELinux and system_server, which do apply there. No health issue changes: `Dex2oatWrapperBroken` is raised only when the state is not DEX2OAT_OK, so it was never raised below Android 10 either.
`FrameworkUpdateViewModel` fetched it into `RootState.version` on every refresh of the framework update screen, and no composable read that field. It was one binder transaction per refresh buying nothing, and it was the only consumer the AIDL method had — so the method had no readers at all. Removed rather than rendered because rendering it is a design decision about that screen, not a defect to repair. The daemon still derives the string and still logs it (`Root implementation: Magisk 27.0 via …`, and the comma-joined list for ROOT_MULTIPLE), so nothing is lost from a bug report; only the unread IPC is gone. Removing a method shifts the transaction id of every one below it, which is exactly what PROTOCOL_VERSION is there to make safe: a peer that has not been rebuilt is refused at the handshake rather than left calling a number that now means something else.
…was refused `logs_rotate_failed` read "The daemon refused to start a new log", and there is no path by which the daemon refuses. `startNewLogPart` answers nothing — it writes a sentinel into the log for the native reader to act on afterwards — so the only failure the manager can observe is that the transaction never arrived: a dead binder, a descriptor that does not match, or no daemon at all. The inaccuracy predates this branch. `clearLogs` returned a boolean that was the constant `true`, so its false branch already meant "the transaction did not arrive" while the string said the daemon had refused. It sent a reader looking for a permission or a configuration problem when the framework was simply not reachable. A new key rather than a reworded one, because the meaning changes and eighteen translations would otherwise go on asserting the old one. Each translation keeps the word for the daemon that its own file already used — service, background process, 守护进程 — so only the claim changes.
JingMatrix
force-pushed
the
manager-aidl-namespace
branch
from
August 4, 2026 03:25
3833ad0 to
6057399
Compare
The daemon carried NeoZygisk's version floors — 26402 for Magisk, 10762 for APatch — and answered ROOT_TOO_OLD below them, which the update screen rendered as "too old to flash through". That was never the daemon's decision to make. Whether the zygisk loader runs on this device is settled before the daemon exists; a daemon that is answering at all has already passed whatever check the loader applies. Duplicating the floor here only created a second opinion that could disagree with the first, and it had to be kept in step with a number living in another project. The KernelSU branch already said as much: it applies no floor, because `ksud -V` prints a build hash and there is nothing to compare — and that was sound, not a gap. So detection reports what it found and quotes the version back for the user to read. ROOT_TOO_OLD goes, with the string and the demo scenario that rendered it, and the remaining ROOT_* values close up behind it. ROOT_UNKNOWN keeps 0, which it holds because 0 is what a binder proxy returns for a transaction the daemon does not implement. The prose loses its running commentary on NeoZygisk's internals along the way. What is worth saying is why detection goes by binary — the binary has to exist to do the flashing anyway — not how a different project reaches the same answer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finishes the namespace move #757 started. That PR moved the daemon's AIDL and listed three things it left behind: the manager's interface, the
Utilslogger, and the daemon's implementation class names. This does all three./data/adb/lspdis deliberately untouched — it is on-disk state and needs a migration, not a rename.ILSPManagerServiceand its parcelables becomeIManagerService,IFrameworkInstallReceiver,ScopeEntryandDeviceUserunderorg.matrix.vector.ipc.org.lsposed.lspd.util.Utilsbecomesorg.matrix.vector.util, with the innerLogpromoted to top level.ApplicationServiceandModuleServicetake the names of the interfaces they implement.The interface goes from 45 methods to 39. Three had no callers,
restartForamong them, which the daemon implemented as a no-op.getUnloadableModulesandgetModuleLoadStatecollapse into one call returning a map: the old pair sent a list of package names and then spent a transaction per name asking why each had failed, so the caller pre-filled every entry withMODULE_LOAD_NO_APK, and a dropped transaction told the user the framework could not find their module's APK.The interface name is the binder descriptor, so this breaks a manager installed as a separate app. Such a copy survives a flash today; after this it binds successfully, then fails every transaction while the binder stays alive, and every screen goes blank with nothing said. The manager now compares descriptors before binding and reports the mismatch.
getProtocolVersiongoes in alongside it, because a new descriptor is the only point at which it needs no fallback of its own. Recovery is the parasitic manager, which ships with the daemon, and that is worth a release note.Seven defects surfaced while documenting the implementation, each with its own commit:
enableModulediscarded the result of itsinsert()and reported a row that was never written as a success;systemServerRequestedlatched before the registration it reports had succeeded;uninstallPackagewaited on a binder thread with no timeout; the dex2oat row read "Supported" on releases that have no wrapper;Log.mutedwas ignored by three overloads; a failed log rotation blamed the daemon for refusing; andgetRootImplementationVersionwas fetched on every refresh and never rendered.