[Proposal] Fix breadcrumb race condition

firebase-crashlytics/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+- [fixed] Fixed race condition that caused logs from background threads to not be attached to 
+  reports in some cases [#8034]
+
 # 20.0.4
 
 - [changed] Updated `firebase-sessions` dependency to v3.0.4

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java

@@ -16,6 +16,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
@@ -458,6 +459,31 @@ public void testBreadcrumbSourceIsRegistered() {
     Mockito.verify(mockBreadcrumbSource).registerBreadcrumbHandler(any(BreadcrumbHandler.class));
   }
 
+  /**
+   * Regression test for https://github.com/firebase/firebase-android-sdk/issues/8034.
+   *
+   * <p>Verifies that a breadcrumb logged immediately before {@code logException} is written to disk
+   * before the non-fatal event snapshots the log. Prior to the fix, {@code log()} used
+   * {@code common.submit()} which completed as soon as the disk-write was enqueued, allowing the
+   * subsequent {@code logException()} task to read an empty log. The fix uses {@code submitTask()}
+   * so the common worker suspends until the disk write finishes.
+   */
+  @Test
+  public void testLog_breadcrumbIsWrittenBeforeLogExceptionReadsIt() throws Exception {
+    final String breadcrumb = "test breadcrumb";
+
+    crashlyticsCore.log(breadcrumb);
+    crashlyticsCore.logException(new RuntimeException("non-fatal"), Map.of());
+
+    // Awaiting common is sufficient: submitTask makes common suspend until diskWrite completes,
+    // so when this returns the log entry is guaranteed to be on disk.
+    crashlyticsWorkers.common.await();
+
+    final String logString = crashlyticsCore.getController().getLogString();
+    assertNotNull("Log should have been written before logException read it", logString);
+    assertTrue("Log should contain the breadcrumb", logString.contains(breadcrumb));
+  }
+
   @Test
   public void testOnPreExecute_nativeDidCrashOnPreviousExecution() throws Exception {
     appRestart(); // Create a previous execution

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

@@ -25,6 +25,7 @@
 import android.util.Base64;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import com.google.android.gms.tasks.SuccessContinuation;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.TaskCompletionSource;
@@ -793,6 +794,12 @@ UserMetadata getUserMetadata() {
     return userMetadata;
   }
 
+  @VisibleForTesting
+  @Nullable
+  String getLogString() {
+    return logFileManager.getLogString();
+  }
+
   boolean isHandlingException() {
     return crashHandler != null && crashHandler.isHandlingException();
   }

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

@@ -330,11 +330,10 @@ public void logException(@NonNull Throwable throwable, @NonNull Map<String, Stri
    */
   public void log(final String msg) {
     final long timestamp = System.currentTimeMillis() - startTime;
-    // queuing up on common worker to maintain the order
-    crashlyticsWorkers.common.submit(
-        () -> {
-          crashlyticsWorkers.diskWrite.submit(() -> controller.writeToLog(timestamp, msg));
-        });
+    // submitTask ensures the common worker waits for the diskWrite task to complete, ensuring
+    // that subsequent tasks on the common worker (e.g. logException) see this log entry.
+    crashlyticsWorkers.common.submitTask(
+        () -> crashlyticsWorkers.diskWrite.submit(() -> controller.writeToLog(timestamp, msg)));
   }
 
   public void setUserId(String identifier) {