Skip to content

Commit 785f669

Browse files
authored
Don't use recursion in TrimmedThrowableData to avoid stack overflow (#5310)
Don't use recursion in TrimmedThrowableData. This fixes an issue when exceptions with long caused by chains cause a stack overflow. Pair with PolinaGo
1 parent 87c1f02 commit 785f669

File tree

3 files changed

+44
-13
lines changed

3 files changed

+44
-13
lines changed

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/stacktrace/TrimmedThrowableDataTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.crashlytics.internal.stacktrace;
1616

17+
import static com.google.firebase.crashlytics.internal.stacktrace.TrimmedThrowableData.makeTrimmedThrowableData;
1718
import static org.mockito.Mockito.doReturn;
1819
import static org.mockito.Mockito.mock;
1920

@@ -53,7 +54,7 @@ public void testStackTraceIsTrimmed() {
5354
doReturn(mockStackTrace(3)).when(mockException).getStackTrace();
5455

5556
final StackTraceTrimmingStrategy trimmingStrategy = new TruncateStrategy(1);
56-
final TrimmedThrowableData t = new TrimmedThrowableData(mockException, trimmingStrategy);
57+
final TrimmedThrowableData t = makeTrimmedThrowableData(mockException, trimmingStrategy);
5758

5859
assertEquals(1, t.stacktrace.length);
5960
assertEquals(mockException.getStackTrace()[0], t.stacktrace[0]);
@@ -65,7 +66,7 @@ public void testCauseStackTraceIsTrimmed() {
6566
doReturn(mockCause).when(mockException).getCause();
6667

6768
final StackTraceTrimmingStrategy trimmingStrategy = new TruncateStrategy(1);
68-
final TrimmedThrowableData t = new TrimmedThrowableData(mockException, trimmingStrategy);
69+
final TrimmedThrowableData t = makeTrimmedThrowableData(mockException, trimmingStrategy);
6970

7071
assertEquals(1, t.stacktrace.length);
7172
assertEquals(mockException.getStackTrace()[0], t.stacktrace[0]);
@@ -78,7 +79,7 @@ public void testStackTraceIsNotModifiedIfSmallEnough() {
7879
doReturn(mockStackTrace(3)).when(mockException).getStackTrace();
7980

8081
final StackTraceTrimmingStrategy trimmingStrategy = new TruncateStrategy(5);
81-
final TrimmedThrowableData t = new TrimmedThrowableData(mockException, trimmingStrategy);
82+
final TrimmedThrowableData t = makeTrimmedThrowableData(mockException, trimmingStrategy);
8283

8384
assertEquals(3, t.stacktrace.length);
8485
assertTrue(Arrays.equals(mockException.getStackTrace(), t.stacktrace));
@@ -90,7 +91,7 @@ public void testCauseStackTraceIsNotModifiedIfSmallEnough() {
9091
doReturn(mockCause).when(mockException).getCause();
9192

9293
final StackTraceTrimmingStrategy trimmingStrategy = new TruncateStrategy(5);
93-
final TrimmedThrowableData t = new TrimmedThrowableData(mockException, trimmingStrategy);
94+
final TrimmedThrowableData t = makeTrimmedThrowableData(mockException, trimmingStrategy);
9495

9596
assertEquals(3, t.stacktrace.length);
9697
assertTrue(Arrays.equals(mockException.getStackTrace(), t.stacktrace));
@@ -105,7 +106,7 @@ public void testOnlyCauseStackTraceIsTrimmed() {
105106
doReturn(mockCause).when(mockException).getCause();
106107

107108
final StackTraceTrimmingStrategy trimmingStrategy = new TruncateStrategy(4);
108-
final TrimmedThrowableData t = new TrimmedThrowableData(mockException, trimmingStrategy);
109+
final TrimmedThrowableData t = makeTrimmedThrowableData(mockException, trimmingStrategy);
109110

110111
assertEquals(3, t.stacktrace.length);
111112
assertTrue(Arrays.equals(mockException.getStackTrace(), t.stacktrace));

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsReportDataCapture.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
package com.google.firebase.crashlytics.internal.common;
1616

17+
import static com.google.firebase.crashlytics.internal.stacktrace.TrimmedThrowableData.makeTrimmedThrowableData;
18+
1719
import android.app.ActivityManager;
1820
import android.app.ActivityManager.RunningAppProcessInfo;
1921
import android.content.Context;
@@ -97,7 +99,7 @@ public Event captureEventData(
9799
boolean includeAllThreads) {
98100
final int orientation = context.getResources().getConfiguration().orientation;
99101
final TrimmedThrowableData trimmedEvent =
100-
new TrimmedThrowableData(event, stackTraceTrimmingStrategy);
102+
makeTrimmedThrowableData(event, stackTraceTrimmingStrategy);
101103

102104
return Event.builder()
103105
.setType(type)

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/stacktrace/TrimmedThrowableData.java

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
package com.google.firebase.crashlytics.internal.stacktrace;
1616

17+
import androidx.annotation.Nullable;
18+
import java.util.Stack;
19+
1720
/**
1821
* Decorator class that exposes the appropriate APIs for Crashlytics to write crash data, but which
1922
* pre-processes the stack trace to remove unnecessary frames based on the passed-in
@@ -23,14 +26,39 @@ public class TrimmedThrowableData {
2326
public final String localizedMessage;
2427
public final String className;
2528
public final StackTraceElement[] stacktrace;
26-
public final TrimmedThrowableData cause;
29+
@Nullable public final TrimmedThrowableData cause;
30+
31+
private TrimmedThrowableData(
32+
String localizedMessage,
33+
String className,
34+
StackTraceElement[] stacktrace,
35+
@Nullable TrimmedThrowableData cause) {
36+
this.localizedMessage = localizedMessage;
37+
this.className = className;
38+
this.stacktrace = stacktrace;
39+
this.cause = cause;
40+
}
41+
42+
public static TrimmedThrowableData makeTrimmedThrowableData(
43+
Throwable ex, StackTraceTrimmingStrategy trimmingStrategy) {
44+
Stack<Throwable> throwableStack = new Stack<>();
45+
Throwable exCause = ex;
46+
while (exCause != null) {
47+
throwableStack.push(exCause);
48+
exCause = exCause.getCause();
49+
}
2750

28-
public TrimmedThrowableData(Throwable ex, StackTraceTrimmingStrategy trimmingStrategy) {
29-
this.localizedMessage = ex.getLocalizedMessage();
30-
this.className = ex.getClass().getName();
31-
this.stacktrace = trimmingStrategy.getTrimmedStackTrace(ex.getStackTrace());
51+
TrimmedThrowableData trimmedThrowableData = null;
52+
while (!throwableStack.isEmpty()) {
53+
Throwable throwable = throwableStack.pop();
54+
trimmedThrowableData =
55+
new TrimmedThrowableData(
56+
throwable.getLocalizedMessage(),
57+
throwable.getClass().getName(),
58+
trimmingStrategy.getTrimmedStackTrace(throwable.getStackTrace()),
59+
trimmedThrowableData);
60+
}
3261

33-
final Throwable exCause = ex.getCause();
34-
this.cause = (exCause != null) ? new TrimmedThrowableData(exCause, trimmingStrategy) : null;
62+
return trimmedThrowableData;
3563
}
3664
}

0 commit comments

Comments
 (0)