diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java index 7eaacded569..34a1d72a47d 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java @@ -213,9 +213,10 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal() .thenReturn(new TreeSet<>(Collections.singleton(sessionId))); controller.writeNonFatalException(thread, nonFatal); - controller.doCloseSessions(testSettingsProvider); + crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(testSettingsProvider)); crashlyticsWorkers.common.await(); + crashlyticsWorkers.diskWrite.await(); verify(mockSessionReportingCoordinator) .persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong()); @@ -489,6 +490,8 @@ public void testUploadDisabledThenOptOut() throws Exception { await(controller.deleteUnsentReports()); await(task); + crashlyticsWorkers.diskWrite.await(); + verify(mockSessionReportingCoordinator).removeAllReports(); verifyNoMoreInteractions(mockSessionReportingCoordinator); } diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java index ff7f853adff..4a1d399c137 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/metadata/MetaDataStoreTest.java @@ -170,6 +170,7 @@ public void testWriteUserData_escaped() throws Exception { crashlyticsWorkers.diskWrite.await(); UserMetadata userData = UserMetadata.loadFromExistingSession(SESSION_ID_1, fileStore, crashlyticsWorkers); + Thread.sleep(10); assertEquals(ESCAPED.trim(), userData.getUserId()); } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index dfbd2e9b628..5d60f93a9c8 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -205,7 +205,6 @@ public Task call() throws Exception { // We've fatally crashed, so write the marker file that indicates a crash occurred. crashMarker.create(); - reportingCoordinator.persistFatalEvent( ex, thread, currentSessionId, timestampSeconds); @@ -363,8 +362,11 @@ public Task then(@Nullable Boolean send) throws Exception { public Task call() throws Exception { if (!send) { Logger.getLogger().v("Deleting cached crash reports..."); - deleteFiles(listAppExceptionMarkerFiles()); - reportingCoordinator.removeAllReports(); + crashlyticsWorkers.diskWrite.submit( + () -> { + deleteFiles(listAppExceptionMarkerFiles()); + reportingCoordinator.removeAllReports(); + }); unsentReportsHandled.trySetResult(null); return Tasks.forResult(null); } @@ -512,7 +514,7 @@ boolean finalizeSessions(SettingsProvider settingsProvider) { Logger.getLogger().v("Finalizing previously open sessions."); try { - doCloseSessions(true, settingsProvider); + doCloseSessions(true, settingsProvider, true); } catch (Exception e) { Logger.getLogger().e("Unable to finalize previously open sessions.", e); return false; @@ -557,15 +559,19 @@ private void doOpenSession(String sessionIdentifier, Boolean isOnDemand) { reportingCoordinator.onBeginSession(sessionIdentifier, startedAtSeconds); } + // This is only used for exception handler close session (we have another close session in + // background initialization) void doCloseSessions(SettingsProvider settingsProvider) { - doCloseSessions(false, settingsProvider); + doCloseSessions(false, settingsProvider, false); } /** - * Not synchronized/locked. Must be executed from the single thread executor service used by this - * class. + * + * Not synchronized/locked. Must be executed from the executor service runs tasks in serial order */ - private void doCloseSessions(boolean skipCurrentSession, SettingsProvider settingsProvider) { + private void doCloseSessions( + boolean skipCurrentSession, SettingsProvider settingsProvider, boolean isInitProcess) { + CrashlyticsWorkers.checkBackgroundThread(); final int offset = skipCurrentSession ? 1 : 0; // :TODO HW2021 this implementation can be cleaned up. @@ -579,13 +585,15 @@ private void doCloseSessions(boolean skipCurrentSession, SettingsProvider settin final String mostRecentSessionIdToClose = sortedOpenSessions.get(offset); - if (settingsProvider.getSettingsSync().featureFlagData.collectAnrs) { + // We only collect ANR info for finalize report during initialization process + if (isInitProcess && settingsProvider.getSettingsSync().featureFlagData.collectAnrs) { writeApplicationExitInfoEventIfRelevant(mostRecentSessionIdToClose); } else { Logger.getLogger().v("ANR feature disabled."); } - if (nativeComponent.hasCrashDataForSession(mostRecentSessionIdToClose)) { + // We only collect native crash info for finalize report during initialization process + if (isInitProcess && nativeComponent.hasCrashDataForSession(mostRecentSessionIdToClose)) { // We only finalize the current session if it's a Java crash, so only finalize native crash // data when we aren't including current. finalizePreviousNativeSession(mostRecentSessionIdToClose); diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java index 5e35c87ed67..56ff1244439 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/SessionReportingCoordinator.java @@ -354,12 +354,16 @@ private boolean onReportSendComplete(@NonNull Task { + File reportFile = report.getReportFile(); + if (reportFile.delete()) { + Logger.getLogger().d("Deleted report file: " + reportFile.getPath()); + } else { + Logger.getLogger() + .w("Crashlytics could not delete report file: " + reportFile.getPath()); + } + }); return true; } Logger.getLogger() diff --git a/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java b/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java index 026d5ce2280..ed21115c075 100644 --- a/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java +++ b/firebase-crashlytics/src/test/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerRobolectricTest.java @@ -43,6 +43,7 @@ import com.google.firebase.crashlytics.internal.settings.SettingsProvider; import com.google.firebase.inject.Deferred; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.TreeSet; @@ -65,6 +66,8 @@ public class CrashlyticsControllerRobolectricTest { @Mock private SessionReportingCoordinator mockSessionReportingCoordinator; @Mock private DataCollectionArbiter mockDataCollectionArbiter; + private CrashlyticsWorkers crashlyticsWorkers; + private static final CrashlyticsNativeComponent MISSING_NATIVE_COMPONENT = new CrashlyticsNativeComponentDeferredProxy( new Deferred() { @@ -80,17 +83,24 @@ public void setUp() { MockitoAnnotations.openMocks(this); testContext = getApplicationContext(); testFileStore = new FileStore(testContext); + crashlyticsWorkers = + new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking()); } @Test - public void testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist() { + public void testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist() + throws Exception { final String sessionId = "sessionId"; final CrashlyticsController controller = createController(); when(mockSessionReportingCoordinator.listSortedOpenSessionIds()) .thenReturn(new TreeSet<>(Collections.singletonList(sessionId))); mockSettingsProvider(true, false); - controller.doCloseSessions(mockSettingsProvider); + + crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(mockSettingsProvider)); + // cannot use await since it check preconditions if blocking main thread + Thread.sleep(10); + // Since we haven't added any app exit info to the shadow activity manager, there won't exist a // single app exit info, and so this method won't be called. verify(mockSessionReportingCoordinator, never()) @@ -99,7 +109,8 @@ public void testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntE } @Test - public void testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists() { + public void testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists() throws Exception { + final String sessionIdPrevious = "sessionIdPrevious"; final String sessionId = "sessionId"; final CrashlyticsController controller = createController(); // Adds multiple AppExitInfos to confirm that Crashlytics loops through @@ -109,26 +120,30 @@ public void testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists() { List testApplicationExitInfo = getApplicationExitInfoList(); when(mockSessionReportingCoordinator.listSortedOpenSessionIds()) - .thenReturn(new TreeSet<>(Collections.singletonList(sessionId))); + .thenReturn(new TreeSet<>(Arrays.asList(sessionId, sessionIdPrevious))); mockSettingsProvider(true, false); - controller.doCloseSessions(mockSettingsProvider); + crashlyticsWorkers.common.submit(() -> controller.finalizeSessions(mockSettingsProvider)); + // cannot use await since it check preconditions if blocking main thread + Thread.sleep(100); verify(mockSessionReportingCoordinator) .persistRelevantAppExitInfoEvent( - eq(sessionId), + eq(sessionIdPrevious), eq(testApplicationExitInfo), any(LogFileManager.class), any(UserMetadata.class)); } @Test - public void testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo() { + public void testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo() throws Exception { final String sessionId = "sessionId"; final CrashlyticsController controller = createController(); when(mockSessionReportingCoordinator.listSortedOpenSessionIds()) .thenReturn(new TreeSet<>(Collections.singletonList(sessionId))); mockSettingsProvider(false, false); - controller.doCloseSessions(mockSettingsProvider); + crashlyticsWorkers.common.submit(() -> controller.doCloseSessions(mockSettingsProvider)); + // cannot use await since it check preconditions if blocking main thread + Thread.sleep(10); verify(mockSessionReportingCoordinator, never()) .persistRelevantAppExitInfoEvent( eq(sessionId), any(), any(LogFileManager.class), any(UserMetadata.class)); @@ -177,7 +192,7 @@ private CrashlyticsController createController() { MISSING_NATIVE_COMPONENT, mock(AnalyticsEventLogger.class), mock(CrashlyticsAppQualitySessionsSubscriber.class), - new CrashlyticsWorkers(TestOnlyExecutors.background(), TestOnlyExecutors.blocking())); + crashlyticsWorkers); controller.openSession(SESSION_ID); return controller; }