firebase / firebase/firebase-android-sdk
Crashlytics NDK never closes ApplicationExitInfo.getTraceInputStream(), leaking a ParcelFileDescriptor
- Dominant language
- Java
- Stars
- 2.6k
- Forks
- 710
- Avg merge
- 2d 23h
- Merged PRs (30d)
- 34
Description
### [READ] Step 1: Are you in the right place?
Issues filed here should be about bugs in **the code in this repository**. If you have a general
question, need help debugging, or fall into some other category use one of these other channels:
- For general technical questions, post a question on [StackOverflow](http://stackoverflow.com/)
with the firebase tag.
- For general Firebase discussion, use the
[firebase-talk](https://groups.google.com/forum/#!forum/firebase-talk) google group.
- For help troubleshooting your application that does not fall under one of the above categories,
reach out to the personalized [Firebase support channel](https://firebase.google.com/support/).
### [REQUIRED] Step 2: Describe your environment
- Android Studio version: N/A (not tooling related)
- Firebase Component: Crashlytics (NDK)
- Component version: firebase-crashlytics-ndk 19.2.1 (BOM 33.6.0) — but the relevant code is unchanged on `main` as of 285a048, so this affects current versions too
### [REQUIRED] Step 3: Describe the problem
We run our internal builds with a StrictMode VM policy that has `detectLeakedClosableObjects()` enabled and reports violations as non-fatals. After a run that ends in a native crash or ANR, the next app start reliably produces this violation from inside Crashlytics' own session finalization:
```
Caused by dalvik.system.CloseGuard$CloseGuardException: Explicit termination method 'close' not called
at dalvik.system.CloseGuard.openWithCallSite(CloseGuard.java:287)
at dalvik.system.CloseGuard.open(CloseGuard.java:257)
at android.util.CloseGuard.open(CloseGuard.java:153)
at android.os.ParcelFileDescriptor.(ParcelFileDescriptor.java:210)
at android.os.ParcelFileDescriptor$2.createFromParcel(ParcelFileDescriptor.java:1157)
at android.os.ParcelFileDescriptor$2.createFromParcel(ParcelFileDescriptor.java:1148)
at android.os.Parcel.readTypedObject(Parcel.java:4366)
at android.app.IParcelFileDescriptorRetriever$Stub$Proxy.getPfd(IParcelFileDescriptorRetriever.java:129)
at android.app.ApplicationExitInfo.getTraceInputStream(ApplicationExitInfo.java:1000)
at com.google.firebase.crashlytics.ndk.CrashpadController.getTraceFileFromApplicationExitInfo(CrashpadController.java:264)
at com.google.firebase.crashlytics.ndk.CrashpadController.convertApplicationExitInfoToModel(CrashpadController.java:256)
at com.google.firebase.crashlytics.ndk.CrashpadController.getRelevantApplicationExitInfo(CrashpadController.java:164)
at com.google.firebase.crashlytics.ndk.CrashpadController.getNativeCrashApplicationExitInfo(CrashpadController.java:146)
at com.google.firebase.crashlytics.ndk.CrashpadController.getApplicationExitInfo(CrashpadController.java:130)
at com.google.firebase.crashlytics.ndk.CrashpadController.getNativeCore(CrashpadController.java:124)
at com.google.firebase.crashlytics.ndk.CrashpadController.getFilesForSession(CrashpadController.java:111)
at com.google.firebase.crashlytics.ndk.FirebaseCrashlyticsNdk.getSessionFileProvider(FirebaseCrashlyticsNdk.java:97)
at com.google.firebase.crashlytics.internal.CrashlyticsNativeComponentDeferredProxy.getSessionFileProvider(CrashlyticsNativeComponentDeferredProxy.java:79)
at com.google.firebase.crashlytics.internal.common.CrashlyticsController.finalizePreviousNativeSession(CrashlyticsController.java:665)
at com.google.firebase.crashlytics.internal.common.CrashlyticsController.doCloseSessions(CrashlyticsController.java:587)
at com.google.firebase.crashlytics.internal.common.CrashlyticsController.finalizeSessions(CrashlyticsController.java:506)
at com.google.firebase.crashlytics.internal.common.CrashlyticsCore.doBackgroundInitialization(CrashlyticsCore.java:250)
```
The cause is that `getTraceFileFromApplicationExitInfo` reads the stream returned by `ApplicationExitInfo.getTraceInputStream()` to the end but never closes it. That stream is a `GZIPInputStream` wrapping a `ParcelFileDescriptor.AutoCloseInputStream`, so closing it is the only thing that releases the underlying `ParcelFileDescriptor`. Since nothing does, the fd sits around until the finalizer runs and CloseGuard flags it. The stack trace above is the allocation site CloseGuard recorded, which is why every frame is framework/Crashlytics code.
#### Steps to reproduce:
1. App with `firebase-crashlytics-ndk`.
2. Enable `StrictMode.VmPolicy` with `detectLeakedClosableObjects()` and `penaltyLog()` (a penalty listener that records to Crashlytics makes it show up in the console, which is how we noticed).
3. Crash the app natively.
4. Relaunch and wait for Crashlytics background init to finalize the previous session. The violation is logged.
#### Relevant Code:
https://github.com/firebase/firebase-android-sdk/blob/285a0488fa6de3dc0163f09b265ad9f611891f61/firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java#L261-L287
happy to send a PR for this if that’s welcome. It’s a small try-with-resources change, just let me know.
Contributor guide
Research direction
Start in firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java around getTraceFileFromApplicationExitInfo at lines 261-287. Trace how ApplicationExitInfo.getTraceInputStream() is consumed and run the StrictMode reproduction described in the issue. Done means the returned stream and underlying ParcelFileDescriptor are released without the reported CloseGuard violation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, java
- Domain
- mobile-dev, observability
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100