gmscompat: use fresh file proxy fds for app opens#388
Open
inthewaves wants to merge 3 commits into
Open
Conversation
muhomorr
requested changes
Jun 24, 2026
| if (fd != null) { | ||
| String fdPath = "/proc/self/fd/" + fd.getInt$(); | ||
| return new File(fdPath).lastModified(); | ||
| try (ParcelFileDescriptor pfd = openFreshParcelFileDescriptor(path)) { |
Member
There was a problem hiding this comment.
Why was getFileLastModified() switched to using fresh file descriptors?
Member
Author
There was a problem hiding this comment.
GmsCore updates Phenotype shared snapshots by writing to a temp file then renaming it to replace, so caching those fds could be stale. After looking at client code again, I don't think any of them currently use lastModified. I guess we can still just scope the cached path here to Dynamite modules
| return new File(fdPath).lastModified(); | ||
| } | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); |
Member
There was a problem hiding this comment.
File.lastModified() returns 0 on failure.
The GmsCore file proxy only opens files with O_RDONLY. Avoid intercepting write, append or read-write opens so callers keep normal open semantics for unsupported access modes.
inthewaves
force-pushed
the
gmscompat-fix-file-proxy-fds
branch
from
dc5887e to
12c31a8
Compare
Member
Author
|
Sample app output doing |
muhomorr
requested changes
Jun 25, 2026
Member
There was a problem hiding this comment.
Comment is outdated, /gmscompat_fd is used instead of /proc/self/fd
| } | ||
|
|
||
| private static boolean isApkContainerPath(String path) { | ||
| int zipSeparatorIdx = path.indexOf(ZIP_FILE_SEPARATOR); |
Member
There was a problem hiding this comment.
What's the purpose of checking for ZIP_FILE_SEPARATOR here? Strings with ZIP_FILE_SEPARATOR aren't paths in this context.
Member
Author
There was a problem hiding this comment.
I originally changed the checks in GmsDynamiteClientHooks to also use this so both the Dynamite hooks and the lastModified hooks shared the same check, but decided against it and forgot to revert this part, will remove it
Normal Java file opens such as FileInputStream and ParcelFileDescriptor.open() expect each open to return a fresh descriptor owned and closed by the caller. The regular GMS data file hooks instead returned dup()s of the cached descriptor used for Dynamite /gmscompat_fd_N paths. dup() shares the same open file description, so repeated app reads can inherit another reader's offset and parse truncated Phenotype snapshots. Keep the cache only for Dynamite and open fresh read-only proxy fds for normal Java file opens. Maestro / Pixel Buds app errors: ``` E Maestro_kph: Failed to parse snapshot from shared storage for com.google.android.apps.wearables.maestro.companion#com.google.android.apps.wearables.maestro.companion E Maestro_kph: myz: While parsing a protocol message, the input ended unexpectedly in the middle of a field. This could mean either that the input has been truncated or that an embedded message misreported its own length. E Maestro_kph: at mxq.Y(PG:29) E Maestro_kph: at mxq.j(PG:9) E Maestro_kph: at mxq.o(PG:1) E Maestro_kph: at kpy.b(PG:1) E Maestro_kph: at krz.a(PG:377) E Maestro_kph: at fmq.apply(PG:16) E Maestro_kph: at lzn.e(PG:3) E Maestro_kph: at lzo.run(PG:38) E Maestro_kph: at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:520) E Maestro_kph: at java.util.concurrent.FutureTask.run(FutureTask.java:328) E Maestro_kph: at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:323) E Maestro_kph: at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1100) E Maestro_kph: at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) E Maestro_kph: at java.lang.Thread.run(Thread.java:1572) ``` Test: Maestro no longer has parsing errors for Phenotype flags, and uses the fresh fd path Test: An app with `DynamiteModule.load( context, DynamiteModule.PREFER_REMOTE,"com.google.android.gms.tflite_dynamite" )` uses `openCachedFile` and the cached file branch in `getFileLastModified`
inthewaves
force-pushed
the
gmscompat-fix-file-proxy-fds
branch
from
12c31a8 to
98d1729
Compare
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
2 participants
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.
Closes GrapheneOS/os-issue-tracker#7978
Normal Java file opens such as FileInputStream and ParcelFileDescriptor.open() expect each
open to return a fresh descriptor owned and closed by the caller. The regular GMS data file
hooks instead returned dup()s of the cached descriptor used for Dynamite /gmscompat_fd_N
paths. dup() shares the same open file description, so repeated app reads can inherit another
reader's offset and parse truncated Phenotype snapshots. Keep the cache only for Dynamite and
open fresh read-only proxy fds for normal Java file opens.
Maestro / Pixel Buds app errors: