-
Notifications
You must be signed in to change notification settings - Fork 694
Add didANRKillOnPreviousExecution() to FirebaseCrashlytics #8178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
737f069
51cadc6
5a2e568
2d0c7cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,9 @@ public class CrashlyticsCore { | |
| private CrashlyticsFileMarker crashMarker; | ||
| private boolean didCrashOnPreviousExecution; | ||
|
|
||
| private final Object didANROnPreviousExecutionLock = new Object(); | ||
| @Nullable private volatile Boolean didANROnPreviousExecutionCached; | ||
|
|
||
| private CrashlyticsController controller; | ||
| private final IdManager idManager; | ||
| private final FileStore fileStore; | ||
|
|
@@ -197,6 +200,10 @@ public boolean onPreExecute(AppData appData, SettingsProvider settingsProvider) | |
|
|
||
| checkForPreviousCrash(); | ||
|
|
||
| // Must run before enableExceptionHandling and finalizeSessions, since both alter the | ||
| // set of open sessions. didANROnPreviousExecution() uses the captured ID later. | ||
| controller.capturePreviousExecutionSessionId(); | ||
|
|
||
| controller.enableExceptionHandling( | ||
| sessionIdentifier, Thread.getDefaultUncaughtExceptionHandler(), settingsProvider); | ||
|
|
||
|
|
@@ -502,6 +509,33 @@ public boolean didCrashOnPreviousExecution() { | |
| return didCrashOnPreviousExecution; | ||
| } | ||
|
|
||
| public boolean didANROnPreviousExecution() { | ||
| Boolean cached = didANROnPreviousExecutionCached; | ||
| if (cached != null) { | ||
| return cached; | ||
| } | ||
| synchronized (didANROnPreviousExecutionLock) { | ||
| if (didANROnPreviousExecutionCached != null) { | ||
| return didANROnPreviousExecutionCached; | ||
| } | ||
| Future<Boolean> future = | ||
| crashlyticsWorkers | ||
| .common | ||
| .getExecutor() | ||
| .submit(() -> controller.didANROnPreviousExecution()); | ||
|
|
||
| Boolean result; | ||
| try { | ||
| result = future.get(DEFAULT_MAIN_HANDLER_TIMEOUT_SEC, TimeUnit.SECONDS); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We cannot block the main thread, even when calling this method. The Crashlytics SDK should already know if an ANR happened because it attaches ANRs to the session on initialization without blocking the main thread. Please take a look at how that's done, and model this after that |
||
| } catch (Exception ex) { | ||
| Logger.getLogger().v("Error checking for previous ANR: " + ex.getMessage()); | ||
| result = false; | ||
| } | ||
| didANROnPreviousExecutionCached = Boolean.TRUE.equals(result); | ||
| return didANROnPreviousExecutionCached; | ||
| } | ||
| } | ||
|
|
||
| // endregion | ||
|
|
||
| // region Static utilities | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot change the API. We need this functionality to work on
didCrashOnPreviousExecutionso it returns true for ANRs just like it would for JVM crashes and native crashes