diff --git a/firebase-crashlytics-ndk/CHANGELOG.md b/firebase-crashlytics-ndk/CHANGELOG.md index 3cfdbdd06a0..181e792e258 100644 --- a/firebase-crashlytics-ndk/CHANGELOG.md +++ b/firebase-crashlytics-ndk/CHANGELOG.md @@ -1,5 +1,9 @@ # Unreleased +- [fixed] Fixed a file descriptor leak when reading the native crash trace from + `ApplicationExitInfo`, which could trigger a StrictMode `CloseGuard` violation on the app start + following a native crash. (#8510) + # 20.1.0 - [changed] Updated `firebase-crashlytics` dependency to 20.1.0 diff --git a/firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java b/firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java index 22326f1ddfb..bae5c3eb2d2 100644 --- a/firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java +++ b/firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashpadController.java @@ -257,11 +257,15 @@ private static CrashlyticsReport.ApplicationExitInfo convertApplicationExitInfoT .build(); } + @VisibleForTesting @RequiresApi(api = Build.VERSION_CODES.S) - private static String getTraceFileFromApplicationExitInfo( - ApplicationExitInfo applicationExitInfo) { - try { - return convertInputStreamToString(applicationExitInfo.getTraceInputStream()); + static String getTraceFileFromApplicationExitInfo(ApplicationExitInfo applicationExitInfo) { + // The stream returned by getTraceInputStream() wraps a ParcelFileDescriptor that is only + // released when the stream is closed. Leaving it open leaks the file descriptor until the + // finalizer runs, which StrictMode's detectLeakedClosableObjects() reports as a CloseGuard + // violation. try-with-resources is null safe, so a missing trace still returns null. + try (InputStream traceInputStream = applicationExitInfo.getTraceInputStream()) { + return convertInputStreamToString(traceInputStream); } catch (IOException e) { Logger.getLogger().w("Failed to get input stream from ApplicationExitInfo"); } diff --git a/firebase-crashlytics-ndk/src/test/java/com/google/firebase/crashlytics/ndk/CrashpadControllerRobolectricTest.java b/firebase-crashlytics-ndk/src/test/java/com/google/firebase/crashlytics/ndk/CrashpadControllerRobolectricTest.java new file mode 100644 index 00000000000..356f9518a88 --- /dev/null +++ b/firebase-crashlytics-ndk/src/test/java/com/google/firebase/crashlytics/ndk/CrashpadControllerRobolectricTest.java @@ -0,0 +1,172 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.firebase.crashlytics.ndk; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import android.app.ApplicationExitInfo; +import android.os.Build.VERSION_CODES; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.zip.GZIPInputStream; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +/** + * Tests that the trace stream taken from an {@link ApplicationExitInfo} is always released. + * + *
The stream returned by {@link ApplicationExitInfo#getTraceInputStream()} wraps a {@code + * ParcelFileDescriptor}, so failing to close it leaks a file descriptor until finalization and trips + * StrictMode's {@code detectLeakedClosableObjects()}. See + * https://github.com/firebase/firebase-android-sdk/issues/8510. + */ +@RunWith(RobolectricTestRunner.class) +@Config(sdk = VERSION_CODES.TIRAMISU) +public class CrashpadControllerRobolectricTest { + + private static final String TRACE = "----- pid 1234 -----\nnative crash trace\n----- end -----\n"; + + @Test + public void getTraceFileFromApplicationExitInfo_closesTraceInputStream() throws IOException { + CloseTrackingInputStream traceInputStream = + new CloseTrackingInputStream( + new ByteArrayInputStream(TRACE.getBytes(StandardCharsets.UTF_8))); + ApplicationExitInfo applicationExitInfo = mock(ApplicationExitInfo.class); + when(applicationExitInfo.getTraceInputStream()).thenReturn(traceInputStream); + + String traceFile = CrashpadController.getTraceFileFromApplicationExitInfo(applicationExitInfo); + + assertTrue("The trace input stream must be closed.", traceInputStream.isClosed()); + assertEquals(TRACE, gunzipAndDecode(traceFile)); + } + + @Test + public void getTraceFileFromApplicationExitInfo_closesTraceInputStreamWhenReadFails() + throws IOException { + ThrowingInputStream traceInputStream = new ThrowingInputStream(); + ApplicationExitInfo applicationExitInfo = mock(ApplicationExitInfo.class); + when(applicationExitInfo.getTraceInputStream()).thenReturn(traceInputStream); + + String traceFile = CrashpadController.getTraceFileFromApplicationExitInfo(applicationExitInfo); + + assertNull(traceFile); + assertTrue( + "The trace input stream must be closed even when reading it fails.", + traceInputStream.isClosed()); + } + + @Test + public void getTraceFileFromApplicationExitInfo_nullTraceInputStream_returnsNull() + throws IOException { + ApplicationExitInfo applicationExitInfo = mock(ApplicationExitInfo.class); + when(applicationExitInfo.getTraceInputStream()).thenReturn(null); + + assertNull(CrashpadController.getTraceFileFromApplicationExitInfo(applicationExitInfo)); + } + + @Test + public void getTraceFileFromApplicationExitInfo_traceInputStreamThrows_returnsNull() + throws IOException { + ApplicationExitInfo applicationExitInfo = mock(ApplicationExitInfo.class); + when(applicationExitInfo.getTraceInputStream()).thenThrow(new IOException("no trace")); + + assertNull(CrashpadController.getTraceFileFromApplicationExitInfo(applicationExitInfo)); + } + + /** The caller owns the stream, so this helper must keep leaving it open. */ + @Test + public void convertInputStreamToString_roundTripsWithoutClosingTheStream() throws IOException { + CloseTrackingInputStream inputStream = + new CloseTrackingInputStream( + new ByteArrayInputStream(TRACE.getBytes(StandardCharsets.UTF_8))); + + String converted = CrashpadController.convertInputStreamToString(inputStream); + + assertEquals(TRACE, gunzipAndDecode(converted)); + assertFalse(inputStream.isClosed()); + } + + @Test + public void convertInputStreamToString_nullInputStream_returnsNull() throws IOException { + assertNull(CrashpadController.convertInputStreamToString(null)); + } + + /** Reverses {@code CrashpadController}'s gzip + base64 encoding of the trace file. */ + private static String gunzipAndDecode(String traceFile) throws IOException { + byte[] gzipped = Base64.getDecoder().decode(traceFile); + try (GZIPInputStream gzip = new GZIPInputStream(new ByteArrayInputStream(gzipped)); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + byte[] bytes = new byte[8192]; + int length; + while ((length = gzip.read(bytes)) != -1) { + out.write(bytes, 0, length); + } + return out.toString(StandardCharsets.UTF_8.name()); + } + } + + private static final class CloseTrackingInputStream extends FilterInputStream { + private boolean closed = false; + + CloseTrackingInputStream(InputStream inputStream) { + super(inputStream); + } + + boolean isClosed() { + return closed; + } + + @Override + public void close() throws IOException { + closed = true; + super.close(); + } + } + + private static final class ThrowingInputStream extends InputStream { + private boolean closed = false; + + boolean isClosed() { + return closed; + } + + @Override + public int read() throws IOException { + throw new IOException("Failed to read the trace"); + } + + @Override + public int read(byte[] bytes, int offset, int length) throws IOException { + throw new IOException("Failed to read the trace"); + } + + @Override + public void close() { + closed = true; + } + } +}