Repository navigation
fix(crashlytics,android): read the ELF build ID off the main thread, once per process - #18835
Conversation
…once per process onAttachedToEngine read libapp.so's build ID on the main thread for every engine. With minSdk >= 24 the native libraries are not extracted, so that opened base.apk and its splits as zip files during startup, which is jank and a StrictMode disk-read violation, repeated for each background engine. Read it on first use instead, in recordError, which already runs on the cached thread pool, and share the result across engines.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
Reading it on the first `recordError` meant the read could happen while the app is short of memory or file descriptors, often the reason it reports an error, and a failed read was cached for the whole process. It is now read once when the plugin attaches, on the thread pool, so the first report doesn't wait for it either.
demolaf
left a comment
demolaf
left a comment
There was a problem hiding this comment.
Can you update the description to match the second commit? It still says the build ID is read "on first use" in recordError and names onAttachedToEngine_doesNotReadLibappOnTheMainThread, but the code now reads it at startup in the background and the test is onAttachedToEngine_readsLibappOnceInTheBackground.
… seam internal - A read of libapp.so that fails (memory or file descriptor pressure) is no longer cached, so a later report retries it. A library without a build ID (debug builds) is still read only once. - `buildIdExecutor` is internal instead of a public `setBuildIdExecutor`, and the plugin test moves to Kotlin to use it.
|
@demolaf Thanks for the review. I updated the description to match the current code: the build ID is read once at startup on |
Description
FlutterFirebaseCrashlyticsPlugin.onAttachedToEngineread the ELF build ID oflibapp.so(ElfBuildIdReader.readBuildId) on the main thread, every time an engine attached. With minSdk >= 24 the native libraries are not extracted (extractNativeLibs=falseis the AGP default), sonativeLibraryDir/libapp.sodoes not exist and the reader opensbase.apk, then each split, as aZipFileto look forlib/<abi>/libapp.so. That is disk I/O during app startup on the main thread, a StrictMode violation, and it is repeated for every background engine (messaging, workmanager, ...).Reproduced on the Pixel 9a emulator with the crashlytics example (debug build, minSdk 24) and a local, uncommitted
StrictMode.ThreadPolicy(detectDiskReads().penaltyLog()) inMainActivity.onCreate:mainFlutterFirebaseCrashlyticsPlugin.onAttachedToEngine->ElfBuildIdReader.readBuildId->readBuildIdFromApk->ZipFile(access,open,fstat,lseek,read,mmapof the central directory)Now:
onAttachedToEnginestarts the read oncachedThreadPooland returns. Reading at startup rather than in the firstrecordErrorkeeps it out of the moment the app is failing, which is often when it is short of memory or file descriptors.ElfBuildIdReader.cachedBuildIdreadslibapp.soonce per process and shares the result between engines: the file cannot change while the process runs.OutOfMemoryError) is not cached, so a laterrecordErrorretries it. A library without a build ID, such as a debug (JIT) build with nolibapp.so, is cached, so the APK is not scanned again for every report.recordError, which already runs oncachedThreadPool, still prefers the ELF build ID over the Dart snapshot one, as before.The executor used for the startup read is an
internalproperty, so tests can replace it without adding to the plugin's public API.Tests
JUnit, run by
android-unit-tests(:firebase_crashlytics:testDebugUnitTest), newFlutterFirebaseCrashlyticsPluginTest(Kotlin,src/test/kotlin):onAttachedToEngine_readsLibappOnceInTheBackground: attaches the plugin to two engines with a context that records which threads look upapplicationInfo(where every read starts). Checks that no read happens on the attaching thread, thatlibapp.sois read once for both engines, and that the cached ID is the library's. Fails onmain(read on the attaching thread, once per engine).cachedBuildId_readsLibappOncePerProcess: three lookups from two engines readlibapp.soonce and return its build ID. Fails if the cache is removed (3 reads).cachedBuildId_readsAgainAfterAFailedRead: a first read that throws returnsnull, and the next one returns the build ID. Fails if failures are cached.cachedBuildId_keepsALibraryWithoutBuildId: an APK withoutlibapp.sois scanned once, not on every lookup.The existing
ElfBuildIdReaderTesthelpers that build an ELF image are now package-private so the new test can reuse them.Related Issues
None found.
Checklist
///).melos run analyze) does not report any problems on my PR.Breaking Change