From ea3546761f831ba8145416b24dd86f1c5a4f9a73 Mon Sep 17 00:00:00 2001 From: Mila <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 May 2026 13:38:42 -0400 Subject: [PATCH 1/4] fix(common): Resolve thread deadlock in HeartBeatInfoStorage Remove the legacy `synchronized` keyword from private helper methods and pure utility methods inside `HeartBeatInfoStorage` (such as `getStoredUserAgentString`, `updateStoredUserAgent`, `removeStoredDate`, `getFormattedDate`, `cleanUpStoredHeartBeats`, and `isSameDateUtc`). Under the bug, `storeHeartBeat()` locked the class monitor while blocking in `runBlocking` for Jetpack DataStore's `editSync()` to complete. Because DataStore schedules transactions on a background thread, helper methods called inside the transaction block that were also synchronized on the same monitor resulted in a permanent deadlock. Since these helper methods operate exclusively on thread-local transaction parameters and do not access shared mutable instance state, they do not require synchronization. Add regression unit test `storeHeartBeat_whenCalledOnSeparateThread_doesNotDeadlock` to verify the fix. Fixes #8016 --- .../heartbeatinfo/HeartBeatInfoStorage.java | 12 ++-- .../HeartBeatInfoStorageTest.java | 71 +++++++++++++++++++ 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorage.java b/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorage.java index d998729c69d..7f5484e0d77 100644 --- a/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorage.java +++ b/firebase-common/src/main/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorage.java @@ -128,7 +128,7 @@ synchronized List getAllHeartBeats() { return heartBeatResults; } - private synchronized Preferences.Key> getStoredUserAgentString( + private Preferences.Key> getStoredUserAgentString( MutablePreferences preferences, String dateString) { for (Map.Entry, Object> entry : preferences.asMap().entrySet()) { if (entry.getValue() instanceof Set) { @@ -143,7 +143,7 @@ private synchronized Preferences.Key> getStoredUserAgentString( return null; } - private synchronized void updateStoredUserAgent( + private void updateStoredUserAgent( MutablePreferences preferences, Preferences.Key> userAgent, String dateString) { removeStoredDate(preferences, dateString); Set userAgentDateSet = @@ -152,7 +152,7 @@ private synchronized void updateStoredUserAgent( preferences.set(userAgent, userAgentDateSet); } - private synchronized void removeStoredDate(MutablePreferences preferences, String dateString) { + private void removeStoredDate(MutablePreferences preferences, String dateString) { // Find stored heartbeat and clear it. Preferences.Key> userAgent = getStoredUserAgentString(preferences, dateString); if (userAgent == null) { @@ -179,7 +179,7 @@ synchronized void postHeartBeatCleanUp() { }); } - private synchronized String getFormattedDate(long millis) { + private String getFormattedDate(long millis) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { Instant instant = new Date(millis).toInstant(); LocalDateTime ldt = instant.atOffset(ZoneOffset.UTC).toLocalDateTime(); @@ -226,7 +226,7 @@ synchronized void storeHeartBeat(long millis, String userAgentString) { }); } - private synchronized long cleanUpStoredHeartBeats(MutablePreferences preferences) { + private long cleanUpStoredHeartBeats(MutablePreferences preferences) { long heartBeatCount = JavaDataStorageKt.getOrDefault(preferences, HEART_BEAT_COUNT_TAG, 0L); String lowestDate = null; @@ -264,7 +264,7 @@ synchronized void updateGlobalHeartBeat(long millis) { }); } - synchronized boolean isSameDateUtc(long base, long target) { + boolean isSameDateUtc(long base, long target) { return getFormattedDate(base).equals(getFormattedDate(target)); } diff --git a/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java b/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java index 6a56ef231d1..033b5119fff 100644 --- a/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java +++ b/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java @@ -15,8 +15,13 @@ package com.google.firebase.heartbeatinfo; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import android.content.Context; +import androidx.datastore.preferences.core.MutablePreferences; import androidx.datastore.preferences.core.Preferences; import androidx.datastore.preferences.core.PreferencesKeys; import androidx.test.core.app.ApplicationProvider; @@ -26,6 +31,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.CompletableFuture; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -238,4 +244,69 @@ public void shouldSendGlobalHeartBeat_answerIsYes() { assertThat(heartBeatInfoStorage.shouldSendGlobalHeartBeat(currentTime)).isTrue(); assertThat(heartBeatDataStore.getSync(GLOBAL, -1L)).isEqualTo(currentTime); } + + /** + * Regression test for https://github.com/firebase/firebase-android-sdk/issues/8016 + * + *

Root Cause: The synchronized storeHeartBeat() method locks the HeartBeatInfoStorage + * instance while waiting for JavaDataStorage.editSync(...) to complete. However, editSync + * schedules its preference updates on a different background thread. If helper methods called + * inside the editSync transaction block (such as getStoredUserAgentString, updateStoredUserAgent, + * or cleanUpStoredHeartBeats) are also marked as synchronized, the background thread blocks + * trying to acquire the HeartBeatInfoStorage lock, which is held by the caller thread waiting + * for the background thread to complete, causing a permanent deadlock. + * + *

Fix: Removed the synchronized keyword from helper methods (and isSameDateUtc) since + * they operate exclusively on thread-local transaction parameters and do not access shared mutable + * instance state. + */ + @Test + public void storeHeartBeat_whenCalledOnSeparateThread_doesNotDeadlock() throws Exception { + // Create a mocked JavaDataStorage + JavaDataStorage mockDataStore = mock(JavaDataStorage.class); + HeartBeatInfoStorage heartBeatStorageWithMock = new HeartBeatInfoStorage(mockDataStore); + + // Mock editSync to run the transform on a background thread and block the caller thread + doAnswer( + invocation -> { + kotlin.jvm.functions.Function1 transform = + invocation.getArgument(0); + + CompletableFuture future = + CompletableFuture.runAsync( + () -> { + MutablePreferences mockPrefs = mock(MutablePreferences.class); + // Mock get(LAST_STORED_DATE) to return the target date to force entry into + // the if block + when(mockPrefs.get(PreferencesKeys.stringKey("last-used-date"))) + .thenReturn("1970-01-01"); + // Mock asMap() to avoid NullPointerException + when(mockPrefs.asMap()).thenReturn(Collections.emptyMap()); + + transform.invoke(mockPrefs); + }); + + future.get(); // Blocks the caller thread + return mock(Preferences.class); + }) + .when(mockDataStore) + .editSync(any()); + + // Spawn a thread to call storeHeartBeat, which should deadlock under the bug + Thread thread = + new Thread( + () -> { + heartBeatStorageWithMock.storeHeartBeat(0L, "test-agent"); // 1970-01-01 + }); + + thread.start(); + thread.join(3000); // Wait 3 seconds + + // Since the bug is fixed, the thread should not be alive. + try { + assertThat(thread.isAlive()).isFalse(); + } finally { + thread.interrupt(); + } + } } From c5611a26bd5e64470cf2c0e1d1eb3a17dc3023ef Mon Sep 17 00:00:00 2001 From: Mila <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 May 2026 13:44:13 -0400 Subject: [PATCH 2/4] Update CHANGELOG.md --- firebase-common/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/firebase-common/CHANGELOG.md b/firebase-common/CHANGELOG.md index ed9082936bd..b16178ee1cd 100644 --- a/firebase-common/CHANGELOG.md +++ b/firebase-common/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- [fixed] Resolved a thread deadlock in HeartBeatInfoStorage when using Jetpack DataStore background executors.(#8182) + # 22.0.1 - [changed] Improve datastore support (#7277) From eb6557bed8321017cbabfe316b60e3506e161ef2 Mon Sep 17 00:00:00 2001 From: Mila <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 May 2026 13:44:13 -0400 Subject: [PATCH 3/4] Update CHANGELOG.md --- firebase-common/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/firebase-common/CHANGELOG.md b/firebase-common/CHANGELOG.md index ed9082936bd..b16178ee1cd 100644 --- a/firebase-common/CHANGELOG.md +++ b/firebase-common/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- [fixed] Resolved a thread deadlock in HeartBeatInfoStorage when using Jetpack DataStore background executors.(#8182) + # 22.0.1 - [changed] Improve datastore support (#7277) From d19ec04508fe333a78828bda18b84faa3d41993e Mon Sep 17 00:00:00 2001 From: Mila <107142260+milaGGL@users.noreply.github.com> Date: Tue, 19 May 2026 18:11:37 -0400 Subject: [PATCH 4/4] test(common): Migrate deadlock regression test to Kotlin --- .../HeartBeatInfoStorageKotlinTest.kt | 99 +++++++++++++++++++ .../HeartBeatInfoStorageTest.java | 71 ------------- 2 files changed, 99 insertions(+), 71 deletions(-) create mode 100644 firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageKotlinTest.kt diff --git a/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageKotlinTest.kt b/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageKotlinTest.kt new file mode 100644 index 00000000000..705dea4bb80 --- /dev/null +++ b/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageKotlinTest.kt @@ -0,0 +1,99 @@ +/* + * 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.heartbeatinfo + +import androidx.datastore.preferences.core.MutablePreferences +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.stringPreferencesKey +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import com.google.firebase.datastorage.JavaDataStorage +import java.util.Collections +import java.util.concurrent.CompletableFuture +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.doAnswer +import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` + +@RunWith(AndroidJUnit4::class) +class HeartBeatInfoStorageKotlinTest { + + /** + * Regression test for https://github.com/firebase/firebase-android-sdk/issues/8016 + * + *

Root Cause: The synchronized storeHeartBeat() method locks the HeartBeatInfoStorage instance + * while waiting for JavaDataStorage.editSync(...) to complete. However, editSync schedules its + * preference updates on a different background thread. If helper methods called inside the + * editSync transaction block (such as getStoredUserAgentString, updateStoredUserAgent, or + * cleanUpStoredHeartBeats) are also marked as synchronized, the background thread blocks trying + * to acquire the HeartBeatInfoStorage lock, which is held by the caller thread waiting for the + * background thread to complete, causing a permanent deadlock. + * + *

Fix: Removed the synchronized keyword from helper methods (and isSameDateUtc) since they + * operate exclusively on thread-local transaction parameters and do not access shared mutable + * instance state. + */ + @Test + fun storeHeartBeat_whenCalledOnSeparateThread_doesNotDeadlock() { + val mockDataStore = mock(JavaDataStorage::class.java) + val heartBeatStorageWithMock = HeartBeatInfoStorage(mockDataStore) + + // Mock editSync to run the transform on a background thread and block the caller thread + doAnswer { invocation -> + @Suppress("UNCHECKED_CAST") + val transform = invocation.getArgument<(MutablePreferences) -> Unit>(0) + + val future = + CompletableFuture.runAsync { + val mockPrefs = mock(MutablePreferences::class.java) + // Mock get(LAST_STORED_DATE) to return the target date to force entry into the if block + `when`(mockPrefs.get(stringPreferencesKey("last-used-date"))).thenReturn("1970-01-01") + // Mock asMap() to avoid NullPointerException + `when`(mockPrefs.asMap()).thenReturn(Collections.emptyMap()) + + transform(mockPrefs) + } + + future.get() // Blocks the caller thread + mock(Preferences::class.java) + } + .`when`(mockDataStore) + .editSync(anyTransform()) + + // Spawn a thread to call storeHeartBeat, which would deadlock under the bug + val thread = Thread { + heartBeatStorageWithMock.storeHeartBeat(0L, "test-agent") // 1970-01-01 + } + + thread.start() + thread.join(3000) // Wait 3 seconds + + try { + // Since the bug is fixed, the thread should not be alive. + assertThat(thread.isAlive).isFalse() + } finally { + thread.interrupt() + } + } + + private fun anyTransform(): (MutablePreferences) -> Unit { + any>() + return { _: MutablePreferences -> } + } +} diff --git a/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java b/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java index 033b5119fff..6a56ef231d1 100644 --- a/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java +++ b/firebase-common/src/test/java/com/google/firebase/heartbeatinfo/HeartBeatInfoStorageTest.java @@ -15,13 +15,8 @@ package com.google.firebase.heartbeatinfo; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import android.content.Context; -import androidx.datastore.preferences.core.MutablePreferences; import androidx.datastore.preferences.core.Preferences; import androidx.datastore.preferences.core.PreferencesKeys; import androidx.test.core.app.ApplicationProvider; @@ -31,7 +26,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.Set; -import java.util.concurrent.CompletableFuture; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -244,69 +238,4 @@ public void shouldSendGlobalHeartBeat_answerIsYes() { assertThat(heartBeatInfoStorage.shouldSendGlobalHeartBeat(currentTime)).isTrue(); assertThat(heartBeatDataStore.getSync(GLOBAL, -1L)).isEqualTo(currentTime); } - - /** - * Regression test for https://github.com/firebase/firebase-android-sdk/issues/8016 - * - *

Root Cause: The synchronized storeHeartBeat() method locks the HeartBeatInfoStorage - * instance while waiting for JavaDataStorage.editSync(...) to complete. However, editSync - * schedules its preference updates on a different background thread. If helper methods called - * inside the editSync transaction block (such as getStoredUserAgentString, updateStoredUserAgent, - * or cleanUpStoredHeartBeats) are also marked as synchronized, the background thread blocks - * trying to acquire the HeartBeatInfoStorage lock, which is held by the caller thread waiting - * for the background thread to complete, causing a permanent deadlock. - * - *

Fix: Removed the synchronized keyword from helper methods (and isSameDateUtc) since - * they operate exclusively on thread-local transaction parameters and do not access shared mutable - * instance state. - */ - @Test - public void storeHeartBeat_whenCalledOnSeparateThread_doesNotDeadlock() throws Exception { - // Create a mocked JavaDataStorage - JavaDataStorage mockDataStore = mock(JavaDataStorage.class); - HeartBeatInfoStorage heartBeatStorageWithMock = new HeartBeatInfoStorage(mockDataStore); - - // Mock editSync to run the transform on a background thread and block the caller thread - doAnswer( - invocation -> { - kotlin.jvm.functions.Function1 transform = - invocation.getArgument(0); - - CompletableFuture future = - CompletableFuture.runAsync( - () -> { - MutablePreferences mockPrefs = mock(MutablePreferences.class); - // Mock get(LAST_STORED_DATE) to return the target date to force entry into - // the if block - when(mockPrefs.get(PreferencesKeys.stringKey("last-used-date"))) - .thenReturn("1970-01-01"); - // Mock asMap() to avoid NullPointerException - when(mockPrefs.asMap()).thenReturn(Collections.emptyMap()); - - transform.invoke(mockPrefs); - }); - - future.get(); // Blocks the caller thread - return mock(Preferences.class); - }) - .when(mockDataStore) - .editSync(any()); - - // Spawn a thread to call storeHeartBeat, which should deadlock under the bug - Thread thread = - new Thread( - () -> { - heartBeatStorageWithMock.storeHeartBeat(0L, "test-agent"); // 1970-01-01 - }); - - thread.start(); - thread.join(3000); // Wait 3 seconds - - // Since the bug is fixed, the thread should not be alive. - try { - assertThat(thread.isAlive()).isFalse(); - } finally { - thread.interrupt(); - } - } }