Fix the work profile the package list drops and the module that cannot reach it - #964
Open
JingMatrix wants to merge 3 commits into
Open
Fix the work profile the package list drops and the module that cannot reach it#964JingMatrix wants to merge 3 commits into
JingMatrix wants to merge 3 commits into
Conversation
getInstalledPackagesFromAllUsers asks system_server for every package,
once per user, and accumulates the answers. getInstalledPackagesReflect
turned any failure of that query into an empty list, and the loop reads
an empty list as a user with nothing installed and moves on -- so a
transaction that died came back as a device where that profile holds no
apps at all, which the manager then draws as fact.
The kernel says what actually fails, and it is not one oversized reply.
A report on a Samsung A52 with a Shelter work profile has 409 of
binder_alloc: 1519: binder_alloc_buf size 197464 failed, no address space
binder_alloc: allocated: 463248 (num: 532 largest: 235880),
free: 577136 (num: 114 largest: 118688)
against pid 1519, the daemon: 577 KB free but no block larger than
118 KB for a 197 KB reply, with 532 buffers outstanding and peaks at
687. system_server then fails the reply with -28, ENOSPC, and the
DeadObjectException the daemon sees says "remote process probably died,
but this could also be caused by running out of binder buffer" -- there
is no tombstone for system_server anywhere near it, so it is the second
clause. The daemon's binder heap is a fixed megabyte and roughly sixty
manager threads were enumerating packages into it at once; one
enumeration on its own is fine, and the same device answers a correct
831 when it is not racing itself.
The manager side of that is a separate commit. What is wrong here is
that the daemon translates ENOSPC into "this user has no packages", and
the caller has no way to tell the difference. v2.0 did not:
PackageService.getInstalledPackagesFromAllUsers was declared throws
RemoteException and called getInstalledPackages straight, so a dead
transaction failed the whole call. The Kotlin rewrite wrapped it in
runCatching and answered emptyList(), and the flags are unchanged either
side -- v2.0's MATCH_ALL_FLAGS already carried MATCH_ANY_USER -- so the
swallow is the whole of the regression.
So the reflection answers null for a query that did not happen, which is
not the same answer as a user holding nothing, and the caller refuses to
build a list it knows is short.
A module reaches a profile's apps only from inside that profile: the scope list is filtered to the module's own user, because that is the boundary the daemon enforces. So a module the owner holds and the work profile does not can never be scoped there, and the manager -- which runs in one user -- had nothing that could put it in the other. The module list then hides a profile with no modules in it, which is right on its own but closes the circle: the profile is invisible because it has no module, and no module can be sent to it. Android keeps one APK per package name for the whole device and varies only who has it installed, so this is not a copy. It is the platform's own installExistingPackageAsUser, the same call pm install-existing makes, which flips the install state for that user and nothing else -- the version, the signature and the scope rows are all untouched. Offered only where it can do something: on a module, and only for the users that do not already hold it. Those are read off the module list's own per-user tabs rather than asked of the daemon, so the row cannot disagree with the list behind it, and a user with no tab holds nothing and is exactly the user this exists to reach. A device with one user never sees the row. The users are listed rather than confirmed, because the question is which user, and on a device with both a work profile and a private space there is no sensible one to preselect.
f93a416 gave AppRepository a job that concurrent readers join rather than each starting their own, because enumerating packages through the daemon is expensive in a way its call site does not look. The Modules panel was left out of it: discover() called daemonClient.getInstalledPackagesFromAllUsers directly, with no cache and nothing to coalesce two of them. It is the worst place to have left out. The scan is driven by packageRevision, which observePackageChanges bumps from two merged sources -- the platform's broadcast and the daemon's re-broadcast -- so one install starts two enumerations by design, and a profile whose packages churn keeps many more than two alive. A report on a Samsung A52 with a Shelter work profile has 148 scans in eight minutes, five inside one second, across 59 manager threads. The daemon's binder heap is a fixed megabyte, and that many chunked ParceledListSlices in flight together do not fit in it: binder_alloc: 1519: binder_alloc_buf size 197464 failed, no address space binder_alloc: allocated: 463248 (num: 532 largest: 235880), free: 577136 (num: 114 largest: 118688) 409 of those, against the daemon, with 687 buffers outstanding at the peak. Note the shape of it: 577 KB free and not one block big enough for a 197 KB reply. It is saturation and fragmentation, not a reply that was ever too large -- one enumeration answers a correct 831 packages on the same device, and the failures only start once the scans pile up. What that costs is not time. A per-user query answered ENOSPC used to take that user out of the list, so the same device reported 27 different totals between 0 and 831, and 697 of them -- 831 less the work profile -- is a reader being told they have no work profile. The commit before this one stops the daemon lying about that; this one stops the manager asking sixty times at once. A second enumeration rather than a filter over getInstalledApps: this one needs MATCH_ANY_USER and uninstalled packages so a module held only by a profile is still seen, and must not set filterNoProcess, because a module with no components of its own is still a module. It answers a Result, so a failed read stays distinguishable from a device with no packages, and only a success is cached.
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.
getInstalledPackagesFromAllUsersis a union over users: it askssystem_serverfor each user's packages and adds them to a running list. A union cannot distinguish a term that is genuinely empty from one that failed — both arrive as no rows, both shrink the total. That matters only if the failing term can be made to answer nothing.The Modules panel rescans on every package event, and
observePackageChangesmerges the platform's broadcast with the daemon's re-broadcast, so one install fires two.discover()called the daemon directly, not through the shared job f93a416 gaveAppRepository, so nothing coalesced them:148 of those in eight minutes, across 59 threads.
Each scan is many transactions:
ParceledListSlicehands back a binder the receiver calls repeatedly, and every callback needs a contiguous buffer in its mapping —BINDER_VM_SIZE, a megabyte less two pages.463248 + 577136is exactly one mebibyte less eight kilobytes: the daemon's whole binder region, full. 577 KB free across 114 blocks, largest 118 KB, against a 197 KB request — fragmentation as much as exhaustion. 409 of those, peaking at 687 outstanding buffers.The exception names two causes and it is always the second; 32 tombstones, none in the window. The reflection answered
emptyList(), soENOSPCon one user deleted that user: 27 totals between 0 and 831, where 697 is 831 less user 10's 134 packages. v2.0 declaredthrows RemoteExceptionand propagated; the Kotlin refactor's swallow is the regression.So the reflection answers
nullfor a query that did not happen, the caller refuses a union missing a term, and the scan becomes a shared job like the app list.Also here: a module can be put into a user that does not hold it. Scope is filtered to the module's own user, so a module a profile lacks could never be scoped there.
One gap: a failed enumeration still draws "No modules installed.", since
daemonAvailablecomes fromgetUsers().Closes #953.