Crashlytics NDK never closes ApplicationExitInfo.getTraceInputStream(), leaking a ParcelFileDescriptor

[prempalsingh]

[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
  with the firebase tag.
• For general Firebase discussion, use the
  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.

[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.<init>(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:

firebase-android-sdk/firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java

Lines 261 to 287 in 285a048

	private static String getTraceFileFromApplicationExitInfo(
	ApplicationExitInfo applicationExitInfo) {
	try {
	return convertInputStreamToString(applicationExitInfo.getTraceInputStream());
	} catch (IOException e) {
	Logger.getLogger().w("Failed to get input stream from ApplicationExitInfo");
	}
	return null;
	}
	@VisibleForTesting
	@RequiresApi(api = Build.VERSION_CODES.S)
	public static String convertInputStreamToString(InputStream inputStream) throws IOException {
	if (inputStream == null) {
	return null;
	}
	ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
	byte[] bytes = new byte[8192];
	int length;
	while ((length = inputStream.read(bytes)) != -1) {
	byteArrayOutputStream.write(bytes, 0, length);
	}
	return zipAndEncode(byteArrayOutputStream.toByteArray());
	}

happy to send a PR for this if that’s welcome. It’s a small try-with-resources change, just let me know.

[lehcar09]

HI @prempalsingh, thank you for reaching out. I was able to reproduce the issue. I'll raise this to our engineers. Please feel free to submit a PR. We really appreciate developers who contributes that helps us improve our services. Thanks!