From 2a0d6da985ad283700ae95598c42e74915951254 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 12:34:21 +0200 Subject: [PATCH 01/43] perf(connectivity): Cache network capabilities and status to reduce IPC calls --- .../core/AndroidOptionsInitializer.java | 4 +- .../util/AndroidConnectionStatusProvider.java | 443 ++++++++++++--- .../AndroidConnectionStatusProviderTest.kt | 515 ++++++++++++++---- sentry/api/sentry.api | 3 +- .../io/sentry/IConnectionStatusProvider.java | 3 +- .../sentry/NoOpConnectionStatusProvider.java | 6 + sentry/src/main/java/io/sentry/Scopes.java | 1 + .../test/java/io/sentry/SentryOptionsTest.kt | 2 + 8 files changed, 819 insertions(+), 158 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 302f4627f20..c3303cf4698 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -28,6 +28,7 @@ import io.sentry.android.core.internal.gestures.AndroidViewGestureTargetLocator; import io.sentry.android.core.internal.modules.AssetsModulesLoader; import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider; +import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.android.core.performance.AppStartMetrics; @@ -157,7 +158,8 @@ static void initializeIntegrationsAndProcessors( if (options.getConnectionStatusProvider() instanceof NoOpConnectionStatusProvider) { options.setConnectionStatusProvider( - new AndroidConnectionStatusProvider(context, options.getLogger(), buildInfoProvider)); + new AndroidConnectionStatusProvider( + context, options, buildInfoProvider, AndroidCurrentDateProvider.getInstance())); } if (options.getCacheDirPath() != null) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index ed8948e0a5a..3df49171d32 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -8,12 +8,15 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.os.Build; +import androidx.annotation.NonNull; import io.sentry.IConnectionStatusProvider; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; import io.sentry.SentryLevel; +import io.sentry.SentryOptions; import io.sentry.android.core.BuildInfoProvider; import io.sentry.android.core.ContextUtils; +import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; @@ -31,104 +34,402 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusProvider { private final @NotNull Context context; - private final @NotNull ILogger logger; + private final @NotNull SentryOptions options; private final @NotNull BuildInfoProvider buildInfoProvider; + private final @NotNull ICurrentDateProvider timeProvider; private final @NotNull List connectionStatusObservers; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); private volatile @Nullable NetworkCallback networkCallback; + private static final @NotNull AutoClosableReentrantLock connectivityManagerLock = + new AutoClosableReentrantLock(); + private static volatile @Nullable ConnectivityManager connectivityManager; + + private static final int[] transports = { + NetworkCapabilities.TRANSPORT_WIFI, + NetworkCapabilities.TRANSPORT_CELLULAR, + NetworkCapabilities.TRANSPORT_ETHERNET + }; + + private static final int[] capabilities = new int[2]; + + private final @NotNull Thread initThread; + private volatile @Nullable NetworkCapabilities cachedNetworkCapabilities; + private volatile @Nullable Network currentNetwork; + private volatile long lastCacheUpdateTime = 0; + private static final long CACHE_TTL_MS = 2 * 60 * 1000L; // 2 minutes + + @SuppressLint("InlinedApi") public AndroidConnectionStatusProvider( @NotNull Context context, - @NotNull ILogger logger, - @NotNull BuildInfoProvider buildInfoProvider) { + @NotNull SentryOptions options, + @NotNull BuildInfoProvider buildInfoProvider, + @NotNull ICurrentDateProvider timeProvider) { this.context = ContextUtils.getApplicationContext(context); - this.logger = logger; + this.options = options; this.buildInfoProvider = buildInfoProvider; + this.timeProvider = timeProvider; this.connectionStatusObservers = new ArrayList<>(); + + capabilities[0] = NetworkCapabilities.NET_CAPABILITY_INTERNET; + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) { + capabilities[1] = NetworkCapabilities.NET_CAPABILITY_VALIDATED; + } + + // Register network callback immediately for caching + //noinspection Convert2MethodRef + initThread = new Thread(() -> ensureNetworkCallbackRegistered()); + initThread.start(); } - @Override - public @NotNull ConnectionStatus getConnectionStatus() { - final ConnectivityManager connectivityManager = getConnectivityManager(context, logger); - if (connectivityManager == null) { - return ConnectionStatus.UNKNOWN; + /** + * Enhanced network connectivity check for Android 15. Checks for NET_CAPABILITY_INTERNET, + * NET_CAPABILITY_VALIDATED, and proper transport types. + * https://medium.com/@doronkakuli/adapting-your-network-connectivity-checks-for-android-15-a-practical-guide-2b1850619294 + */ + @SuppressLint("InlinedApi") + private boolean isNetworkEffectivelyConnected( + final @Nullable NetworkCapabilities networkCapabilities) { + if (networkCapabilities == null) { + return false; + } + + // Check for general internet capability AND validated status + boolean hasInternetAndValidated = + networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) { + hasInternetAndValidated = + hasInternetAndValidated + && networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); } - return getConnectionStatus(context, connectivityManager, logger); - // getActiveNetworkInfo might return null if VPN doesn't specify its - // underlying network - // when min. API 24, use: - // connectivityManager.registerDefaultNetworkCallback(...) + if (!hasInternetAndValidated) { + return false; + } + + // Additionally, ensure it's a recognized transport type for general internet access + return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) + || networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) + || networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET); } - @Override - public @Nullable String getConnectionType() { - return getConnectionType(context, logger, buildInfoProvider); + /** Get connection status from cached NetworkCapabilities or fallback to legacy method. */ + private @NotNull ConnectionStatus getConnectionStatusFromCache() { + if (cachedNetworkCapabilities != null) { + return isNetworkEffectivelyConnected(cachedNetworkCapabilities) + ? ConnectionStatus.CONNECTED + : ConnectionStatus.DISCONNECTED; + } + + // Fallback to legacy method when NetworkCapabilities not available + final ConnectivityManager connectivityManager = + getConnectivityManager(context, options.getLogger()); + if (connectivityManager != null) { + return getConnectionStatus(context, connectivityManager, options.getLogger()); + } + + return ConnectionStatus.UNKNOWN; } - @Override - public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) { - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - connectionStatusObservers.add(observer); + /** Get connection type from cached NetworkCapabilities or fallback to legacy method. */ + private @Nullable String getConnectionTypeFromCache() { + final NetworkCapabilities capabilities = cachedNetworkCapabilities; + if (capabilities != null) { + return getConnectionType(capabilities); } - if (networkCallback == null) { - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - if (networkCallback == null) { - final @NotNull NetworkCallback newNetworkCallback = - new NetworkCallback() { - @Override - public void onAvailable(final @NotNull Network network) { - updateObservers(); - } + // Fallback to legacy method when NetworkCapabilities not available + return getConnectionType(context, options.getLogger(), buildInfoProvider); + } - @Override - public void onUnavailable() { - updateObservers(); - } + private void ensureNetworkCallbackRegistered() { + if (networkCallback != null) { + return; // Already registered + } - @Override - public void onLost(final @NotNull Network network) { - updateObservers(); - } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (networkCallback != null) { + return; + } - public void updateObservers() { - final @NotNull ConnectionStatus status = getConnectionStatus(); - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - for (final @NotNull IConnectionStatusObserver observer : - connectionStatusObservers) { - observer.onConnectionStatusChanged(status); - } + final @NotNull NetworkCallback callback = + new NetworkCallback() { + @Override + public void onAvailable(final @NotNull Network network) { + currentNetwork = network; + } + + @Override + public void onUnavailable() { + clearCacheAndNotifyObservers(); + } + + @Override + public void onLost(final @NotNull Network network) { + if (!network.equals(currentNetwork)) { + return; + } + clearCacheAndNotifyObservers(); + } + + private void clearCacheAndNotifyObservers() { + // Clear cached capabilities and network reference atomically + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + cachedNetworkCapabilities = null; + currentNetwork = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + + options + .getLogger() + .log(SentryLevel.DEBUG, "Cache cleared - network lost/unavailable"); + + // Notify all observers with DISCONNECTED status directly + // No need to query ConnectivityManager - we know the network is gone + for (final @NotNull IConnectionStatusObserver observer : + connectionStatusObservers) { + observer.onConnectionStatusChanged(ConnectionStatus.DISCONNECTED); + } + } + } + + @Override + public void onCapabilitiesChanged( + @NonNull Network network, @NonNull NetworkCapabilities networkCapabilities) { + if (!network.equals(currentNetwork)) { + return; + } + updateCacheAndNotifyObservers(network, networkCapabilities); + } + + private void updateCacheAndNotifyObservers( + @Nullable Network network, @Nullable NetworkCapabilities networkCapabilities) { + // Check if this change is meaningful before notifying observers + final boolean shouldUpdate = isSignificantChange(networkCapabilities); + + // Only notify observers if something meaningful changed + if (shouldUpdate) { + updateCache(networkCapabilities); + + final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + for (final @NotNull IConnectionStatusObserver observer : + connectionStatusObservers) { + observer.onConnectionStatusChanged(status); } } - }; + } + } + + /** + * Check if NetworkCapabilities change is significant for our observers. Only notify for + * changes that affect connectivity status or connection type. + */ + private boolean isSignificantChange(@Nullable NetworkCapabilities newCapabilities) { + final NetworkCapabilities oldCapabilities = cachedNetworkCapabilities; + + // Always significant if transitioning between null and non-null + if ((oldCapabilities == null) != (newCapabilities == null)) { + return true; + } + + // If both null, no change + if (oldCapabilities == null && newCapabilities == null) { + return false; + } + + // Check significant capability changes + if (hasSignificantCapabilityChanges(oldCapabilities, newCapabilities)) { + return true; + } + + // Check significant transport changes + if (hasSignificantTransportChanges(oldCapabilities, newCapabilities)) { + return true; + } + + return false; + } + + /** Check if capabilities that affect connectivity status changed. */ + private boolean hasSignificantCapabilityChanges( + @NotNull NetworkCapabilities old, @NotNull NetworkCapabilities new_) { + // Check capabilities we care about for connectivity determination + for (int capability : capabilities) { + if (capability != 0 + && old.hasCapability(capability) != new_.hasCapability(capability)) { + return true; + } + } + + return false; + } + + /** Check if transport types that affect connection type changed. */ + private boolean hasSignificantTransportChanges( + @NotNull NetworkCapabilities old, @NotNull NetworkCapabilities new_) { + // Check transports we care about for connection type determination + for (int transport : transports) { + if (old.hasTransport(transport) != new_.hasTransport(transport)) { + return true; + } + } + + return false; + } + }; - if (registerNetworkCallback(context, logger, buildInfoProvider, newNetworkCallback)) { - networkCallback = newNetworkCallback; - return true; + if (registerNetworkCallback(context, options.getLogger(), buildInfoProvider, callback)) { + networkCallback = callback; + options.getLogger().log(SentryLevel.DEBUG, "Network callback registered successfully"); + } else { + options.getLogger().log(SentryLevel.WARNING, "Failed to register network callback"); + } + } + } + + @SuppressLint({"NewApi", "MissingPermission"}) + private void updateCache(@Nullable NetworkCapabilities networkCapabilities) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + try { + if (networkCapabilities != null) { + cachedNetworkCapabilities = networkCapabilities; + } else { + if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) { + options + .getLogger() + .log( + SentryLevel.INFO, + "No permission (ACCESS_NETWORK_STATE) to check network status."); + cachedNetworkCapabilities = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + return; + } + + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) { + cachedNetworkCapabilities = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + return; + } + + // Fallback: query current active network + final ConnectivityManager connectivityManager = + getConnectivityManager(context, options.getLogger()); + if (connectivityManager != null) { + final Network activeNetwork = connectivityManager.getActiveNetwork(); + + cachedNetworkCapabilities = + activeNetwork != null + ? connectivityManager.getNetworkCapabilities(activeNetwork) + : null; } else { - return false; + cachedNetworkCapabilities = + null; // Clear cached capabilities if connectivity manager is null } } + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Cache updated - Status: " + + getConnectionStatusFromCache() + + ", Type: " + + getConnectionTypeFromCache()); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.WARNING, "Failed to update connection status cache", t); + cachedNetworkCapabilities = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); } } - // networkCallback is already registered, so we can safely return true - return true; + } + + private boolean isCacheValid() { + return (timeProvider.getCurrentTimeMillis() - lastCacheUpdateTime) < CACHE_TTL_MS; + } + + @Override + public @NotNull ConnectionStatus getConnectionStatus() { + if (!isCacheValid()) { + updateCache(null); + } + return getConnectionStatusFromCache(); + } + + @Override + public @Nullable String getConnectionType() { + if (!isCacheValid()) { + updateCache(null); + } + return getConnectionTypeFromCache(); + } + + @Override + public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + connectionStatusObservers.add(observer); + } + ensureNetworkCallbackRegistered(); + + // Network callback is already registered during initialization + return networkCallback != null; } @Override public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { connectionStatusObservers.remove(observer); - if (connectionStatusObservers.isEmpty()) { - if (networkCallback != null) { - unregisterNetworkCallback(context, logger, networkCallback); - networkCallback = null; - } - } + // Keep the callback registered for caching even if no observers + } + } + + /** Clean up resources - should be called when the provider is no longer needed */ + @Override + public void close() { + try { + options + .getExecutorService() + .submit( + () -> { + final NetworkCallback callbackRef; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + connectionStatusObservers.clear(); + + callbackRef = networkCallback; + networkCallback = null; + + if (callbackRef != null) { + unregisterNetworkCallback(context, options.getLogger(), callbackRef); + } + // Clear cached state + cachedNetworkCapabilities = null; + currentNetwork = null; + lastCacheUpdateTime = 0; + } + try (final @NotNull ISentryLifecycleToken ignored = + connectivityManagerLock.acquire()) { + connectivityManager = null; + } + }); + } catch (Throwable t) { + options + .getLogger() + .log(SentryLevel.ERROR, "Error submitting AndroidConnectionStatusProvider task", t); } } + /** + * Get the cached NetworkCapabilities for advanced use cases. Returns null if cache is stale or no + * capabilities are available. + * + * @return cached NetworkCapabilities or null + */ + @TestOnly + @Nullable + public NetworkCapabilities getCachedNetworkCapabilities() { + return cachedNetworkCapabilities; + } + /** * Return the Connection status * @@ -295,12 +596,22 @@ public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObser private static @Nullable ConnectivityManager getConnectivityManager( final @NotNull Context context, final @NotNull ILogger logger) { - final ConnectivityManager connectivityManager = - (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); - if (connectivityManager == null) { - logger.log(SentryLevel.INFO, "ConnectivityManager is null and cannot check network status"); + if (connectivityManager != null) { + return connectivityManager; + } + + try (final @NotNull ISentryLifecycleToken ignored = connectivityManagerLock.acquire()) { + if (connectivityManager != null) { + return connectivityManager; // Double-checked locking + } + + connectivityManager = + (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); + if (connectivityManager == null) { + logger.log(SentryLevel.INFO, "ConnectivityManager is null and cannot check network status"); + } + return connectivityManager; } - return connectivityManager; } @SuppressLint({"MissingPermission", "NewApi"}) @@ -358,4 +669,10 @@ public List getStatusObservers() { public NetworkCallback getNetworkCallback() { return networkCallback; } + + @TestOnly + @NotNull + public Thread getInitThread() { + return initThread; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt index b15ab4e605d..a362885d635 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt @@ -1,18 +1,27 @@ package io.sentry.android.core +import android.Manifest import android.content.Context import android.content.pm.PackageManager.PERMISSION_DENIED +import android.content.pm.PackageManager.PERMISSION_GRANTED import android.net.ConnectivityManager import android.net.ConnectivityManager.NetworkCallback import android.net.Network import android.net.NetworkCapabilities +import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET +import android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED import android.net.NetworkCapabilities.TRANSPORT_CELLULAR import android.net.NetworkCapabilities.TRANSPORT_ETHERNET import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.net.NetworkInfo import android.os.Build import io.sentry.IConnectionStatusProvider +import io.sentry.ILogger +import io.sentry.SentryOptions import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider +import io.sentry.test.ImmediateExecutorService +import io.sentry.transport.ICurrentDateProvider +import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -21,11 +30,13 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.eq import org.mockito.kotlin.mock -import org.mockito.kotlin.never +import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.times import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever class AndroidConnectionStatusProviderTest { @@ -34,14 +45,24 @@ class AndroidConnectionStatusProviderTest { private lateinit var connectivityManager: ConnectivityManager private lateinit var networkInfo: NetworkInfo private lateinit var buildInfo: BuildInfoProvider + private lateinit var timeProvider: ICurrentDateProvider + private lateinit var options: SentryOptions private lateinit var network: Network private lateinit var networkCapabilities: NetworkCapabilities + private lateinit var logger: ILogger + + private var currentTime = 1000L @BeforeTest fun beforeTest() { contextMock = mock() connectivityManager = mock() - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) + whenever(contextMock.getSystemService(Context.CONNECTIVITY_SERVICE)) + .thenReturn(connectivityManager) + whenever( + contextMock.checkPermission(eq(Manifest.permission.ACCESS_NETWORK_STATE), any(), any()) + ) + .thenReturn(PERMISSION_GRANTED) networkInfo = mock() whenever(connectivityManager.activeNetworkInfo).thenReturn(networkInfo) @@ -54,13 +75,38 @@ class AndroidConnectionStatusProviderTest { networkCapabilities = mock() whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(networkCapabilities) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(networkCapabilities.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + + timeProvider = mock() + whenever(timeProvider.currentTimeMillis).thenAnswer { currentTime } + + logger = mock() + options = SentryOptions() + options.setLogger(logger) + options.executorService = ImmediateExecutorService() + + // Reset current time for each test to ensure cache isolation + currentTime = 1000L - connectionStatusProvider = AndroidConnectionStatusProvider(contextMock, mock(), buildInfo) + connectionStatusProvider = + AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider) + + // Wait for async callback registration to complete + connectionStatusProvider.initThread.join() + } + + @AfterTest + fun `tear down`() { + // clear the cache and ensure proper cleanup + connectionStatusProvider.close() } @Test fun `When network is active and connected with permission, return CONNECTED for isConnected`() { whenever(networkInfo.isConnected).thenReturn(true) + assertEquals( IConnectionStatusProvider.ConnectionStatus.CONNECTED, connectionStatusProvider.connectionStatus, @@ -89,6 +135,8 @@ class AndroidConnectionStatusProviderTest { @Test fun `When network is not active, return DISCONNECTED for isConnected`() { + whenever(connectivityManager.activeNetwork).thenReturn(null) + assertEquals( IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, connectionStatusProvider.connectionStatus, @@ -97,98 +145,86 @@ class AndroidConnectionStatusProviderTest { @Test fun `When ConnectivityManager is not available, return UNKNOWN for isConnected`() { - whenever(contextMock.getSystemService(any())).thenReturn(null) + // First close the existing provider to clean up static state + connectionStatusProvider.close() + + // Create a fresh context mock that returns null for ConnectivityManager + val nullConnectivityContext = mock() + whenever(nullConnectivityContext.getSystemService(any())).thenReturn(null) + whenever( + nullConnectivityContext.checkPermission( + eq(Manifest.permission.ACCESS_NETWORK_STATE), + any(), + any(), + ) + ) + .thenReturn(PERMISSION_GRANTED) + + // Create a new provider with the null connectivity manager + val providerWithNullConnectivity = + AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider) + providerWithNullConnectivity.initThread.join() // Wait for async init to complete + assertEquals( IConnectionStatusProvider.ConnectionStatus.UNKNOWN, - connectionStatusProvider.connectionStatus, + providerWithNullConnectivity.connectionStatus, ) + + providerWithNullConnectivity.close() } @Test fun `When ConnectivityManager is not available, return null for getConnectionType`() { - assertNull(AndroidConnectionStatusProvider.getConnectionType(mock(), mock(), buildInfo)) + whenever(contextMock.getSystemService(any())).thenReturn(null) + assertNull(connectionStatusProvider.connectionType) } @Test fun `When there's no permission, return null for getConnectionType`() { whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + assertNull(connectionStatusProvider.connectionType) } @Test fun `When network is not active, return null for getConnectionType`() { - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) + whenever(connectivityManager.activeNetwork).thenReturn(null) - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + assertNull(connectionStatusProvider.connectionType) } @Test fun `When network capabilities are not available, return null for getConnectionType`() { - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(null) + + assertNull(connectionStatusProvider.connectionType) } @Test fun `When network capabilities has TRANSPORT_WIFI, return wifi`() { whenever(networkCapabilities.hasTransport(eq(TRANSPORT_WIFI))).thenReturn(true) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(false) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(false) - assertEquals( - "wifi", - AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo), - ) + assertEquals("wifi", connectionStatusProvider.connectionType) } @Test fun `When network capabilities has TRANSPORT_ETHERNET, return ethernet`() { whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(true) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_WIFI))).thenReturn(false) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(false) - assertEquals( - "ethernet", - AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo), - ) + assertEquals("ethernet", connectionStatusProvider.connectionType) } @Test fun `When network capabilities has TRANSPORT_CELLULAR, return cellular`() { + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_WIFI))).thenReturn(false) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(false) whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(true) - assertEquals( - "cellular", - AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo), - ) - } - - @Test - fun `When there's no permission, do not register any NetworkCallback`() { - val buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) - val registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - contextMock, - mock(), - buildInfo, - mock(), - ) - - assertFalse(registered) - verify(connectivityManager, never()).registerDefaultNetworkCallback(any()) - } - - @Test - fun `When sdkInfoVersion is not min N, do not register any NetworkCallback`() { - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - val registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - contextMock, - mock(), - buildInfo, - mock(), - ) - - assertFalse(registered) - verify(connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertEquals("cellular", connectionStatusProvider.connectionType) } @Test @@ -199,7 +235,7 @@ class AndroidConnectionStatusProviderTest { val registered = AndroidConnectionStatusProvider.registerNetworkCallback( contextMock, - mock(), + logger, buildInfo, mock(), ) @@ -211,17 +247,16 @@ class AndroidConnectionStatusProviderTest { @Test fun `unregisterNetworkCallback calls connectivityManager unregisterDefaultNetworkCallback`() { whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - AndroidConnectionStatusProvider.unregisterNetworkCallback(contextMock, mock(), mock()) + AndroidConnectionStatusProvider.unregisterNetworkCallback(contextMock, logger, mock()) verify(connectivityManager).unregisterNetworkCallback(any()) } @Test fun `When connectivityManager getActiveNetwork throws an exception, getConnectionType returns null`() { - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.S) whenever(connectivityManager.activeNetwork).thenThrow(SecurityException("Android OS Bug")) - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + assertNull(connectionStatusProvider.connectionType) } @Test @@ -231,27 +266,13 @@ class AndroidConnectionStatusProviderTest { assertFalse( AndroidConnectionStatusProvider.registerNetworkCallback( contextMock, - mock(), + logger, buildInfo, mock(), ) ) } - @Test - fun `When connectivityManager unregisterDefaultCallback throws an exception, it gets swallowed`() { - whenever(connectivityManager.registerDefaultNetworkCallback(any())) - .thenThrow(SecurityException("Android OS Bug")) - - var failed = false - try { - AndroidConnectionStatusProvider.unregisterNetworkCallback(contextMock, mock(), mock()) - } catch (t: Throwable) { - failed = true - } - assertFalse(failed) - } - @Test fun `connectionStatus returns NO_PERMISSIONS when context does not hold the permission`() { whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) @@ -261,12 +282,6 @@ class AndroidConnectionStatusProviderTest { ) } - @Test - fun `connectionStatus returns ethernet when underlying mechanism provides ethernet`() { - whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(true) - assertEquals("ethernet", connectionStatusProvider.connectionType) - } - @Test fun `adding and removing an observer works correctly`() { whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) @@ -279,25 +294,341 @@ class AndroidConnectionStatusProviderTest { connectionStatusProvider.removeConnectionStatusObserver(observer) assertTrue(connectionStatusProvider.statusObservers.isEmpty()) - assertNull(connectionStatusProvider.networkCallback) } @Test - fun `underlying callbacks correctly trigger update`() { + fun `cache TTL works correctly`() { + // Setup: Mock network info to return connected + whenever(networkInfo.isConnected).thenReturn(true) + + // For API level M, the code uses getActiveNetwork() and getNetworkCapabilities() + // Let's track calls to these methods to verify caching behavior + + // Make the first call to establish baseline + val firstResult = connectionStatusProvider.connectionStatus + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, firstResult) + + // Count how many times getActiveNetwork was called so far (includes any initialization calls) + val initialCallCount = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + // Advance time by 1 minute (less than 2 minute TTL) + currentTime += 60 * 1000L + + // Second call should use cache - no additional calls to getActiveNetwork + val secondResult = connectionStatusProvider.connectionStatus + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, secondResult) + + val callCountAfterSecond = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + // Verify no additional calls were made (cache was used) + assertEquals(initialCallCount, callCountAfterSecond, "Second call should use cache") + + // Advance time beyond TTL (total 3 minutes) + currentTime += 2 * 60 * 1000L + + // Third call should refresh cache - should make new calls to getActiveNetwork + val thirdResult = connectionStatusProvider.connectionStatus + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, thirdResult) + + val callCountAfterThird = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + // Verify that new calls were made (cache was refreshed) + assertTrue(callCountAfterThird > callCountAfterSecond, "Third call should refresh cache") + + // All results should be consistent + assertEquals(firstResult, secondResult) + assertEquals(secondResult, thirdResult) + } + + @Test + fun `observers are only notified for significant changes`() { whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Get the callback that was registered + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // IMPORTANT: Set network as current first + callback.onAvailable(network) + + // Create network capabilities for testing + val oldCaps = mock() + whenever(oldCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(oldCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(oldCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(oldCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(oldCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(newCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // First callback with capabilities - should notify + callback.onCapabilitiesChanged(network, oldCaps) + + // Second callback with same significant capabilities - should NOT notify additional times + callback.onCapabilitiesChanged(network, newCaps) + + // Only first change should trigger notification + verify(observer, times(1)).onConnectionStatusChanged(any()) + } - var callback: NetworkCallback? = null - whenever(connectivityManager.registerDefaultNetworkCallback(any())).then { invocation -> - callback = invocation.getArgument(0, NetworkCallback::class.java) as NetworkCallback - Unit - } + @Test + fun `observers are notified when significant capabilities change`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) val observer = mock() connectionStatusProvider.addConnectionStatusObserver(observer) - callback!!.onAvailable(mock()) - callback!!.onUnavailable() - callback!!.onLost(mock()) - connectionStatusProvider.removeConnectionStatusObserver(observer) - verify(observer, times(3)).onConnectionStatusChanged(any()) + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // IMPORTANT: Set network as current first + callback.onAvailable(network) + + val oldCaps = mock() + whenever(oldCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(oldCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(false) // Not validated + whenever(oldCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(oldCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(oldCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) // Now validated + whenever(newCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(newCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + callback.onCapabilitiesChanged(network, oldCaps) + callback.onCapabilitiesChanged(network, newCaps) + + // Should be notified for both changes (validation state changed) + verify(observer, times(2)).onConnectionStatusChanged(any()) + } + + @Test + fun `observers are notified when transport changes`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // IMPORTANT: Set network as current first + callback.onAvailable(network) + + val wifiCaps = mock() + whenever(wifiCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(wifiCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(wifiCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(wifiCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(wifiCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + val cellularCaps = mock() + whenever(cellularCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(cellularCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(cellularCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(false) + whenever(cellularCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + whenever(cellularCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + callback.onCapabilitiesChanged(network, wifiCaps) + callback.onCapabilitiesChanged(network, cellularCaps) + + // Should be notified for both changes (transport changed) + verify(observer, times(2)).onConnectionStatusChanged(any()) + } + + @Test + fun `onLost clears cache and notifies with DISCONNECTED`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // Set current network + callback.onAvailable(network) + + // Lose the network + callback.onLost(network) + + assertNull(connectionStatusProvider.cachedNetworkCapabilities) + verify(observer) + .onConnectionStatusChanged(IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) + } + + @Test + fun `onUnavailable clears cache and notifies with DISCONNECTED`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + callback.onUnavailable() + + assertNull(connectionStatusProvider.cachedNetworkCapabilities) + verify(observer) + .onConnectionStatusChanged(IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) + } + + @Test + fun `onLost for different network is ignored`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + val network1 = mock() + val network2 = mock() + + // Set current network + callback.onAvailable(network1) + + // Lose a different network - should be ignored + callback.onLost(network2) + + verifyNoInteractions(observer) + } + + @Test + fun `isNetworkEffectivelyConnected works correctly for Android 15`() { + // Test case: has internet and validated capabilities with good transport + val goodCaps = mock() + whenever(goodCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(goodCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(goodCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(goodCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(goodCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // Override the mock to return good capabilities + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(goodCaps) + + // Force cache invalidation by advancing time beyond TTL + currentTime += 3 * 60 * 1000L // 3 minutes + + // Should return CONNECTED for good capabilities + assertEquals( + IConnectionStatusProvider.ConnectionStatus.CONNECTED, + connectionStatusProvider.connectionStatus, + ) + + // Test case: missing validated capability + val unvalidatedCaps = mock() + whenever(unvalidatedCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(unvalidatedCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(false) + whenever(unvalidatedCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(unvalidatedCaps) + + // Force cache invalidation again + currentTime += 3 * 60 * 1000L + + assertEquals( + IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, + connectionStatusProvider.connectionStatus, + ) + } + + @Test + fun `API level below M falls back to legacy method`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP) + whenever(networkInfo.isConnected).thenReturn(true) + + assertEquals( + IConnectionStatusProvider.ConnectionStatus.CONNECTED, + connectionStatusProvider.connectionStatus, + ) + } + + @Test + fun `onCapabilitiesChanged updates cache`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // Set network as current first + callback.onAvailable(network) + + // Create initial capabilities - CONNECTED state (wifi + validated) + val initialCaps = mock() + whenever(initialCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(initialCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(initialCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(initialCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(initialCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // First callback with initial capabilities + callback.onCapabilitiesChanged(network, initialCaps) + + // Verify cache contains the initial capabilities + assertEquals(initialCaps, connectionStatusProvider.cachedNetworkCapabilities) + + // Verify initial state - should be CONNECTED with wifi + assertEquals( + IConnectionStatusProvider.ConnectionStatus.CONNECTED, + connectionStatusProvider.connectionStatus, + ) + assertEquals("wifi", connectionStatusProvider.connectionType) + + // Create new capabilities - DISCONNECTED state (cellular but not validated) + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)) + .thenReturn(false) // Not validated = DISCONNECTED + whenever(newCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(false) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // Second callback with changed capabilities + callback.onCapabilitiesChanged(network, newCaps) + + // Verify cache is updated with new capabilities + assertEquals(newCaps, connectionStatusProvider.cachedNetworkCapabilities) + + // Verify that subsequent calls use the updated cache + // Both connection status AND type should change + assertEquals( + IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, + connectionStatusProvider.connectionStatus, + ) + assertEquals("cellular", connectionStatusProvider.connectionType) + + // Verify observer was notified of the changes (both calls should notify since capabilities + // changed significantly) + verify(observer, times(2)).onConnectionStatusChanged(any()) } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 171cd7eaa88..b5dcbe2caf6 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -738,7 +738,7 @@ public final class io/sentry/HubScopesWrapper : io/sentry/IHub { public fun withScope (Lio/sentry/ScopeCallback;)V } -public abstract interface class io/sentry/IConnectionStatusProvider { +public abstract interface class io/sentry/IConnectionStatusProvider : java/io/Closeable { public abstract fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)Z public abstract fun getConnectionStatus ()Lio/sentry/IConnectionStatusProvider$ConnectionStatus; public abstract fun getConnectionType ()Ljava/lang/String; @@ -1452,6 +1452,7 @@ public final class io/sentry/NoOpCompositePerformanceCollector : io/sentry/Compo public final class io/sentry/NoOpConnectionStatusProvider : io/sentry/IConnectionStatusProvider { public fun ()V public fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)Z + public fun close ()V public fun getConnectionStatus ()Lio/sentry/IConnectionStatusProvider$ConnectionStatus; public fun getConnectionType ()Ljava/lang/String; public fun removeConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)V diff --git a/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java b/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java index 1d75098e564..bd897882d7a 100644 --- a/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java +++ b/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java @@ -1,11 +1,12 @@ package io.sentry; +import java.io.Closeable; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public interface IConnectionStatusProvider { +public interface IConnectionStatusProvider extends Closeable { enum ConnectionStatus { UNKNOWN, diff --git a/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java b/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java index a1d66c9115b..765c2c0537d 100644 --- a/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java +++ b/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java @@ -1,5 +1,6 @@ package io.sentry; +import java.io.IOException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,4 +26,9 @@ public boolean addConnectionStatusObserver(@NotNull IConnectionStatusObserver ob public void removeConnectionStatusObserver(@NotNull IConnectionStatusObserver observer) { // no-op } + + @Override + public void close() throws IOException { + // no-op + } } diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index fa1b7c2a81e..4b16d71b931 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -446,6 +446,7 @@ public void close(final boolean isRestarting) { getOptions().getTransactionProfiler().close(); getOptions().getContinuousProfiler().close(true); getOptions().getCompositePerformanceCollector().close(); + getOptions().getConnectionStatusProvider().close(); final @NotNull ISentryExecutorService executorService = getOptions().getExecutorService(); if (isRestarting) { executorService.submit( diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index f132807e428..770f7bea877 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -590,6 +590,8 @@ class SentryOptionsTest { val options = SentryOptions() val customProvider = object : IConnectionStatusProvider { + override fun close() = Unit + override fun getConnectionStatus(): IConnectionStatusProvider.ConnectionStatus { return IConnectionStatusProvider.ConnectionStatus.UNKNOWN } From 14bb2d54660ae01e0bc1e48617a49d369ff242ab Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 17:40:40 +0200 Subject: [PATCH 02/43] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1bb8a6656f..f73d9c22b56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) +- Cache network capabilities and status to reduce IPC calls ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) ## 8.17.0 @@ -3539,7 +3540,7 @@ TBD Packages were released on [bintray](https://dl.bintray.com/getsentry/maven/io/sentry/) > Note: This release marks the unification of the Java and Android Sentry codebases based on the core of the Android SDK (version 2.x). -Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ +> Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ ## 3.0.0-alpha.1 From c82586d72541c20fd17d5157d78a9b8a57e50782 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 17:43:23 +0200 Subject: [PATCH 03/43] Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f73d9c22b56..62c8f5e17d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) -- Cache network capabilities and status to reduce IPC calls ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) +- Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) ## 8.17.0 From 7418781778065cce406b3edeb32d03092995cf1e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 17:45:23 +0200 Subject: [PATCH 04/43] revert --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62c8f5e17d8..29dfad2e336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3540,7 +3540,7 @@ TBD Packages were released on [bintray](https://dl.bintray.com/getsentry/maven/io/sentry/) > Note: This release marks the unification of the Java and Android Sentry codebases based on the core of the Android SDK (version 2.x). -> Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ +Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ ## 3.0.0-alpha.1 From f739d004dea8776c72cd36067b2bd95d928de5dd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 00:00:25 +0200 Subject: [PATCH 05/43] fix(breadcrumbs): Deduplicate battery breadcrumbs --- .../SystemEventsBreadcrumbsIntegration.java | 70 ++++++++++++++--- .../SystemEventsBreadcrumbsIntegrationTest.kt | 77 +++++++++++++++++++ 2 files changed, 137 insertions(+), 10 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 27dcbbbd725..b073ab1a32e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -339,6 +339,43 @@ static final class SystemEventsBroadcastReceiver extends BroadcastReceiver { private final @NotNull Debouncer batteryChangedDebouncer = new Debouncer(AndroidCurrentDateProvider.getInstance(), DEBOUNCE_WAIT_TIME_MS, 0); + // Track previous battery state to avoid duplicate breadcrumbs when values haven't changed + private @Nullable BatteryState previousBatteryState; + + static final class BatteryState { + private final @Nullable Float level; + private final @Nullable Boolean charging; + + BatteryState(final @Nullable Float level, final @Nullable Boolean charging) { + this.level = level; + this.charging = charging; + } + + @Override + public boolean equals(final @Nullable Object other) { + if (!(other instanceof BatteryState)) return false; + BatteryState that = (BatteryState) other; + return isSimilarLevel(level, that.level) && Objects.equals(charging, that.charging); + } + + @Override + public int hashCode() { + // Use rounded level for hash consistency + Float roundedLevel = level != null ? Math.round(level * 100f) / 100f : null; + return Objects.hash(roundedLevel, charging); + } + + private boolean isSimilarLevel(final @Nullable Float level1, final @Nullable Float level2) { + if (level1 == null && level2 == null) return true; + if (level1 == null || level2 == null) return false; + + // Round both levels to 2 decimal places and compare + float rounded1 = Math.round(level1 * 100f) / 100f; + float rounded2 = Math.round(level2 * 100f) / 100f; + return Float.compare(rounded1, rounded2) == 0; + } + } + SystemEventsBroadcastReceiver( final @NotNull IScopes scopes, final @NotNull SentryAndroidOptions options) { this.scopes = scopes; @@ -355,14 +392,29 @@ public void onReceive(final Context context, final @NotNull Intent intent) { return; } + // For battery changes, check if the actual values have changed + @Nullable BatteryState batteryState = null; + if (isBatteryChanged) { + final @Nullable Float currentBatteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); + final @Nullable Boolean currentChargingState = DeviceInfoUtil.isCharging(intent, options); + batteryState = new BatteryState(currentBatteryLevel, currentChargingState); + + // Only create breadcrumb if battery state has actually changed + if (batteryState.equals(previousBatteryState)) { + return; + } + + previousBatteryState = batteryState; + } + + final BatteryState state = batteryState; final long now = System.currentTimeMillis(); try { options .getExecutorService() .submit( () -> { - final Breadcrumb breadcrumb = - createBreadcrumb(now, intent, action, isBatteryChanged); + final Breadcrumb breadcrumb = createBreadcrumb(now, intent, action, state); final Hint hint = new Hint(); hint.set(ANDROID_INTENT, intent); scopes.addBreadcrumb(breadcrumb, hint); @@ -411,7 +463,7 @@ String getStringAfterDotFast(final @Nullable String str) { final long timeMs, final @NotNull Intent intent, final @Nullable String action, - boolean isBatteryChanged) { + final @Nullable BatteryState batteryState) { final Breadcrumb breadcrumb = new Breadcrumb(timeMs); breadcrumb.setType("system"); breadcrumb.setCategory("device.event"); @@ -420,14 +472,12 @@ String getStringAfterDotFast(final @Nullable String str) { breadcrumb.setData("action", shortAction); } - if (isBatteryChanged) { - final Float batteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); - if (batteryLevel != null) { - breadcrumb.setData("level", batteryLevel); + if (batteryState != null) { + if (batteryState.level != null) { + breadcrumb.setData("level", batteryState.level); } - final Boolean isCharging = DeviceInfoUtil.isCharging(intent, options); - if (isCharging != null) { - breadcrumb.setData("charging", isCharging); + if (batteryState.charging != null) { + breadcrumb.setData("charging", batteryState.charging); } } else { final Bundle extras = intent.getExtras(); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 99a80f361c5..d35df7c8444 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -197,6 +197,83 @@ class SystemEventsBreadcrumbsIntegrationTest { verifyNoMoreInteractions(fixture.scopes) } + @Test + fun `battery changes with identical values do not generate breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + val intent1 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80) + putExtra(BatteryManager.EXTRA_SCALE, 100) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + val intent2 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80) + putExtra(BatteryManager.EXTRA_SCALE, 100) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + + // Receive first battery change + sut.receiver!!.onReceive(fixture.context, intent1) + + // Receive second battery change with identical values + sut.receiver!!.onReceive(fixture.context, intent2) + + // should only add the first crumb since values are identical + verify(fixture.scopes) + .addBreadcrumb( + check { + assertEquals(it.data["level"], 80f) + assertEquals(it.data["charging"], true) + }, + anyOrNull(), + ) + verifyNoMoreInteractions(fixture.scopes) + } + + @Test + fun `battery changes with minor level differences do not generate breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + val intent1 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80001) // 80.001% + putExtra(BatteryManager.EXTRA_SCALE, 100000) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + val intent2 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80002) // 80.002% + putExtra(BatteryManager.EXTRA_SCALE, 100000) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + + // Receive first battery change + sut.receiver!!.onReceive(fixture.context, intent1) + + // Receive second battery change with very minor level difference (rounds to same 3 decimal + // places) + sut.receiver!!.onReceive(fixture.context, intent2) + + // should only add the first crumb since both round to 80.000% + verify(fixture.scopes) + .addBreadcrumb( + check { + assertEquals(it.data["level"], 80.001f) + assertEquals(it.data["charging"], true) + }, + anyOrNull(), + ) + verifyNoMoreInteractions(fixture.scopes) + } + @Test fun `Do not crash if registerReceiver throws exception`() { val sut = fixture.getSut() From 833b026812ad6ae9634da7f724cae4234a87d64a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 00:05:03 +0200 Subject: [PATCH 06/43] ref --- .../core/SystemEventsBreadcrumbsIntegration.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index b073ab1a32e..7578ed6bf56 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -387,14 +387,14 @@ public void onReceive(final Context context, final @NotNull Intent intent) { final @Nullable String action = intent.getAction(); final boolean isBatteryChanged = ACTION_BATTERY_CHANGED.equals(action); - // aligning with iOS which only captures battery status changes every minute at maximum - if (isBatteryChanged && batteryChangedDebouncer.checkForDebounce()) { - return; - } - - // For battery changes, check if the actual values have changed @Nullable BatteryState batteryState = null; if (isBatteryChanged) { + if (batteryChangedDebouncer.checkForDebounce()) { + // aligning with iOS which only captures battery status changes every minute at maximum + return; + } + + // For battery changes, check if the actual values have changed final @Nullable Float currentBatteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); final @Nullable Boolean currentChargingState = DeviceInfoUtil.isCharging(intent, options); batteryState = new BatteryState(currentBatteryLevel, currentChargingState); From e4596fffb98c8f07e1b36c60755a39452908b3a0 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 00:28:56 +0200 Subject: [PATCH 07/43] Changelog --- CHANGELOG.md | 1 + .../sentry/android/core/SystemEventsBreadcrumbsIntegration.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29dfad2e336..c2a6fbf4978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) +- Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) ## 8.17.0 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 7578ed6bf56..d681f08429a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -372,7 +372,7 @@ private boolean isSimilarLevel(final @Nullable Float level1, final @Nullable Flo // Round both levels to 2 decimal places and compare float rounded1 = Math.round(level1 * 100f) / 100f; float rounded2 = Math.round(level2 * 100f) / 100f; - return Float.compare(rounded1, rounded2) == 0; + return rounded1 == rounded2; } } From 0153ab5f73c7d213d3062396c7813949028b0cec Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 09:17:38 +0200 Subject: [PATCH 08/43] Fix test --- .../util/AndroidConnectionStatusProvider.java | 5 +++++ .../core/NetworkBreadcrumbsIntegrationTest.kt | 21 +++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 3df49171d32..0ea2e1638ab 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -675,4 +675,9 @@ public NetworkCallback getNetworkCallback() { public Thread getInitThread() { return initThread; } + + @TestOnly + public static void setConnectivityManager(final @Nullable ConnectivityManager cm) { + connectivityManager = cm; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt index fd18e3d75b8..768fe87fbbd 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt @@ -16,9 +16,12 @@ import io.sentry.SentryNanotimeDate import io.sentry.TypeCheckHint import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbConnectionDetail import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbsNetworkCallback +import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import java.util.concurrent.TimeUnit +import kotlin.test.AfterTest +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -47,12 +50,6 @@ class NetworkBreadcrumbsIntegrationTest { var nowMs: Long = 0 val network = mock() - init { - whenever(mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(context.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) - .thenReturn(connectivityManager) - } - fun getSut( enableNetworkEventBreadcrumbs: Boolean = true, buildInfo: BuildInfoProvider = mockBuildInfoProvider, @@ -73,6 +70,18 @@ class NetworkBreadcrumbsIntegrationTest { private val fixture = Fixture() + @BeforeTest + fun `set up`() { + whenever(fixture.mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + whenever(fixture.context.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) + .thenReturn(fixture.connectivityManager) + } + + @AfterTest + fun `tear down`() { + AndroidConnectionStatusProvider.setConnectivityManager(null) + } + @Test fun `When network events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() From c31d64874a182bafced477fc1d32a165441d15f4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 18 Jul 2025 16:18:07 +0200 Subject: [PATCH 09/43] perf(connectivity): Have only one NetworkCallback active at a time --- .../api/sentry-android-core.api | 2 +- .../core/AndroidOptionsInitializer.java | 3 +- .../core/NetworkBreadcrumbsIntegration.java | 109 ++++++------------ .../core/SendCachedEnvelopeIntegration.java | 4 +- .../util/AndroidConnectionStatusProvider.java | 69 ++++++++++- .../core/NetworkBreadcrumbsIntegrationTest.kt | 49 +------- .../AndroidConnectionStatusProviderTest.kt | 53 ++++----- .../android/replay/capture/CaptureStrategy.kt | 25 +++- ...achedEnvelopeFireAndForgetIntegration.java | 4 +- 9 files changed, 159 insertions(+), 159 deletions(-) rename sentry-android-core/src/test/java/io/sentry/android/core/{ => internal/util}/AndroidConnectionStatusProviderTest.kt (96%) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index ea67ed5bcb7..987f13f540f 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -263,7 +263,7 @@ public final class io/sentry/android/core/NdkIntegration : io/sentry/Integration } public final class io/sentry/android/core/NetworkBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { - public fun (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/ILogger;)V + public fun (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;)V public fun close ()V public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index c3303cf4698..e4874b4309b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -382,8 +382,7 @@ static void installDefaultIntegrations( } options.addIntegration(new AppComponentsBreadcrumbsIntegration(context)); options.addIntegration(new SystemEventsBreadcrumbsIntegration(context)); - options.addIntegration( - new NetworkBreadcrumbsIntegration(context, buildInfoProvider, options.getLogger())); + options.addIntegration(new NetworkBreadcrumbsIntegration(context, buildInfoProvider)); if (isReplayAvailable) { final ReplayIntegration replay = new ReplayIntegration(context, CurrentDateProvider.getInstance()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java index 5cfb5df2235..c7f3182b97d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java @@ -12,7 +12,6 @@ import io.sentry.Breadcrumb; import io.sentry.DateUtils; import io.sentry.Hint; -import io.sentry.ILogger; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; import io.sentry.Integration; @@ -33,22 +32,17 @@ public final class NetworkBreadcrumbsIntegration implements Integration, Closeab private final @NotNull Context context; private final @NotNull BuildInfoProvider buildInfoProvider; - private final @NotNull ILogger logger; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); - private volatile boolean isClosed; private @Nullable SentryOptions options; @TestOnly @Nullable volatile NetworkBreadcrumbsNetworkCallback networkCallback; public NetworkBreadcrumbsIntegration( - final @NotNull Context context, - final @NotNull BuildInfoProvider buildInfoProvider, - final @NotNull ILogger logger) { + final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider) { this.context = Objects.requireNonNull(ContextUtils.getApplicationContext(context), "Context is required"); this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required"); - this.logger = Objects.requireNonNull(logger, "ILogger is required"); } @Override @@ -59,78 +53,54 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, "SentryAndroidOptions is required"); - logger.log( - SentryLevel.DEBUG, - "NetworkBreadcrumbsIntegration enabled: %s", - androidOptions.isEnableNetworkEventBreadcrumbs()); - this.options = options; + options + .getLogger() + .log( + SentryLevel.DEBUG, + "NetworkBreadcrumbsIntegration enabled: %s", + androidOptions.isEnableNetworkEventBreadcrumbs()); + if (androidOptions.isEnableNetworkEventBreadcrumbs()) { // The specific error is logged in the ConnectivityChecker method if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) { - logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); + options.getLogger().log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); return; } - try { - options - .getExecutorService() - .submit( - new Runnable() { - @Override - public void run() { - // in case integration is closed before the task is executed, simply return - if (isClosed) { - return; - } - - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - networkCallback = - new NetworkBreadcrumbsNetworkCallback( - scopes, buildInfoProvider, options.getDateProvider()); - - final boolean registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - context, logger, buildInfoProvider, networkCallback); - if (registered) { - logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion("NetworkBreadcrumbs"); - } else { - logger.log( - SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed."); - // The specific error is logged by AndroidConnectionStatusProvider - } - } - } - }); - } catch (Throwable t) { - logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t); + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + networkCallback = + new NetworkBreadcrumbsNetworkCallback( + scopes, buildInfoProvider, options.getDateProvider()); + + final boolean registered = + AndroidConnectionStatusProvider.addNetworkCallback( + context, options.getLogger(), buildInfoProvider, networkCallback); + if (registered) { + options.getLogger().log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion("NetworkBreadcrumbs"); + } else { + options + .getLogger() + .log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed."); + // The specific error is logged by AndroidConnectionStatusProvider + } } } } @Override public void close() throws IOException { - isClosed = true; + final @Nullable ConnectivityManager.NetworkCallback callbackRef; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + callbackRef = networkCallback; + networkCallback = null; + } - try { - Objects.requireNonNull(options, "Options is required") - .getExecutorService() - .submit( - () -> { - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - if (networkCallback != null) { - AndroidConnectionStatusProvider.unregisterNetworkCallback( - context, logger, networkCallback); - logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration removed."); - } - networkCallback = null; - } - }); - } catch (Throwable t) { - logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t); + if (callbackRef != null) { + AndroidConnectionStatusProvider.removeNetworkCallback(callbackRef); } } @@ -138,8 +108,6 @@ static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager final @NotNull IScopes scopes; final @NotNull BuildInfoProvider buildInfoProvider; - @Nullable Network currentNetwork = null; - @Nullable NetworkCapabilities lastCapabilities = null; long lastCapabilityNanos = 0; final @NotNull SentryDateProvider dateProvider; @@ -156,21 +124,14 @@ static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager @Override public void onAvailable(final @NonNull Network network) { - if (network.equals(currentNetwork)) { - return; - } final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_AVAILABLE"); scopes.addBreadcrumb(breadcrumb); - currentNetwork = network; lastCapabilities = null; } @Override public void onCapabilitiesChanged( final @NonNull Network network, final @NonNull NetworkCapabilities networkCapabilities) { - if (!network.equals(currentNetwork)) { - return; - } final long nowNanos = dateProvider.now().nanoTimestamp(); final @Nullable NetworkBreadcrumbConnectionDetail connectionDetail = getNewConnectionDetails( @@ -195,12 +156,8 @@ public void onCapabilitiesChanged( @Override public void onLost(final @NonNull Network network) { - if (!network.equals(currentNetwork)) { - return; - } final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_LOST"); scopes.addBreadcrumb(breadcrumb); - currentNetwork = null; lastCapabilities = null; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java index 41f4f838bf5..8eea2d71047 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -75,7 +75,9 @@ public void close() throws IOException { @Override public void onConnectionStatusChanged( final @NotNull IConnectionStatusProvider.ConnectionStatus status) { - if (scopes != null && options != null) { + if (scopes != null + && options != null + && status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) { sendCachedEnvelopes(scopes, options); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 0ea2e1638ab..40e96eeb922 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -9,6 +9,7 @@ import android.net.NetworkCapabilities; import android.os.Build; import androidx.annotation.NonNull; +import androidx.annotation.RequiresApi; import io.sentry.IConnectionStatusProvider; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; @@ -45,6 +46,10 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP new AutoClosableReentrantLock(); private static volatile @Nullable ConnectivityManager connectivityManager; + private static final @NotNull AutoClosableReentrantLock childCallbacksLock = + new AutoClosableReentrantLock(); + private static final @NotNull List childCallbacks = new ArrayList<>(); + private static final int[] transports = { NetworkCapabilities.TRANSPORT_WIFI, NetworkCapabilities.TRANSPORT_CELLULAR, @@ -157,11 +162,24 @@ private void ensureNetworkCallbackRegistered() { @Override public void onAvailable(final @NotNull Network network) { currentNetwork = network; + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onAvailable(network); + } + } } + @RequiresApi(Build.VERSION_CODES.O) @Override public void onUnavailable() { clearCacheAndNotifyObservers(); + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onUnavailable(); + } + } } @Override @@ -170,6 +188,12 @@ public void onLost(final @NotNull Network network) { return; } clearCacheAndNotifyObservers(); + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onLost(network); + } + } } private void clearCacheAndNotifyObservers() { @@ -199,6 +223,12 @@ public void onCapabilitiesChanged( return; } updateCacheAndNotifyObservers(network, networkCapabilities); + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onCapabilitiesChanged(network, networkCapabilities); + } + } } private void updateCacheAndNotifyObservers( @@ -406,6 +436,9 @@ public void close() { currentNetwork = null; lastCacheUpdateTime = 0; } + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.clear(); + } try (final @NotNull ISentryLifecycleToken ignored = connectivityManagerLock.acquire()) { connectivityManager = null; @@ -614,8 +647,35 @@ public NetworkCapabilities getCachedNetworkCapabilities() { } } + public static boolean addNetworkCallback( + final @NotNull Context context, + final @NotNull ILogger logger, + final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull NetworkCallback networkCallback) { + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) { + logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); + return false; + } + + if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) { + logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status."); + return false; + } + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.add(networkCallback); + } + return true; + } + + public static void removeNetworkCallback(final @NotNull NetworkCallback networkCallback) { + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.remove(networkCallback); + } + } + @SuppressLint({"MissingPermission", "NewApi"}) - public static boolean registerNetworkCallback( + static boolean registerNetworkCallback( final @NotNull Context context, final @NotNull ILogger logger, final @NotNull BuildInfoProvider buildInfoProvider, @@ -642,7 +702,7 @@ public static boolean registerNetworkCallback( } @SuppressLint("NewApi") - public static void unregisterNetworkCallback( + static void unregisterNetworkCallback( final @NotNull Context context, final @NotNull ILogger logger, final @NotNull NetworkCallback networkCallback) { @@ -677,7 +737,8 @@ public Thread getInitThread() { } @TestOnly - public static void setConnectivityManager(final @Nullable ConnectivityManager cm) { - connectivityManager = cm; + @NotNull + public static List getChildCallbacks() { + return childCallbacks; } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt index 768fe87fbbd..cbea3499d85 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt @@ -1,8 +1,6 @@ package io.sentry.android.core import android.content.Context -import android.net.ConnectivityManager -import android.net.ConnectivityManager.NetworkCallback import android.net.Network import android.net.NetworkCapabilities import android.os.Build @@ -17,7 +15,6 @@ import io.sentry.TypeCheckHint import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbConnectionDetail import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbsNetworkCallback import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider -import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import java.util.concurrent.TimeUnit import kotlin.test.AfterTest @@ -32,7 +29,6 @@ import org.mockito.kotlin.KInOrder import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check -import org.mockito.kotlin.eq import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -46,7 +42,6 @@ class NetworkBreadcrumbsIntegrationTest { var options = SentryAndroidOptions() val scopes = mock() val mockBuildInfoProvider = mock() - val connectivityManager = mock() var nowMs: Long = 0 val network = mock() @@ -64,7 +59,7 @@ class NetworkBreadcrumbsIntegrationTest { SentryNanotimeDate(DateUtils.nanosToDate(nowNanos), nowNanos) } } - return NetworkBreadcrumbsIntegration(context, buildInfo, options.logger) + return NetworkBreadcrumbsIntegration(context, buildInfo) } } @@ -73,13 +68,11 @@ class NetworkBreadcrumbsIntegrationTest { @BeforeTest fun `set up`() { whenever(fixture.mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(fixture.context.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) - .thenReturn(fixture.connectivityManager) } @AfterTest fun `tear down`() { - AndroidConnectionStatusProvider.setConnectivityManager(null) + AndroidConnectionStatusProvider.getChildCallbacks().clear() } @Test @@ -88,7 +81,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager).registerDefaultNetworkCallback(any()) + assertFalse(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNotNull(sut.networkCallback) } @@ -98,7 +91,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -110,7 +103,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -121,21 +114,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) sut.close() - verify(fixture.connectivityManager).unregisterNetworkCallback(any()) - assertNull(sut.networkCallback) - } - - @Test - fun `When NetworkBreadcrumbsIntegration is closed, it's ignored if not on Android N+`() { - val buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.M) - val sut = fixture.getSut(buildInfo = buildInfo) - assertNull(sut.networkCallback) - - sut.register(fixture.scopes, fixture.options) - sut.close() - - verify(fixture.connectivityManager, never()).unregisterNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -503,22 +482,6 @@ class NetworkBreadcrumbsIntegrationTest { } } - @Test - fun `If integration is opened and closed immediately it still properly unregisters`() { - val executor = DeferredExecutorService() - val sut = fixture.getSut(executor = executor) - - sut.register(fixture.scopes, fixture.options) - sut.close() - - executor.runAll() - - assertNull(sut.networkCallback) - verify(fixture.connectivityManager, never()) - .registerDefaultNetworkCallback(any()) - verify(fixture.connectivityManager, never()).unregisterNetworkCallback(any()) - } - private fun KInOrder.verifyBreadcrumbInOrder( check: (detail: NetworkBreadcrumbConnectionDetail) -> Unit ) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt similarity index 96% rename from sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt rename to sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index a362885d635..6769ac15301 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -1,4 +1,4 @@ -package io.sentry.android.core +package io.sentry.android.core.internal.util import android.Manifest import android.content.Context @@ -18,7 +18,7 @@ import android.os.Build import io.sentry.IConnectionStatusProvider import io.sentry.ILogger import io.sentry.SentryOptions -import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider +import io.sentry.android.core.BuildInfoProvider import io.sentry.test.ImmediateExecutorService import io.sentry.transport.ICurrentDateProvider import kotlin.test.AfterTest @@ -37,6 +37,7 @@ import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class AndroidConnectionStatusProviderTest { @@ -68,7 +69,7 @@ class AndroidConnectionStatusProviderTest { whenever(connectivityManager.activeNetworkInfo).thenReturn(networkInfo) buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.M) + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) network = mock() whenever(connectivityManager.activeNetwork).thenReturn(network) @@ -173,12 +174,6 @@ class AndroidConnectionStatusProviderTest { providerWithNullConnectivity.close() } - @Test - fun `When ConnectivityManager is not available, return null for getConnectionType`() { - whenever(contextMock.getSystemService(any())).thenReturn(null) - assertNull(connectionStatusProvider.connectionType) - } - @Test fun `When there's no permission, return null for getConnectionType`() { whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) @@ -227,23 +222,6 @@ class AndroidConnectionStatusProviderTest { assertEquals("cellular", connectionStatusProvider.connectionType) } - @Test - fun `registerNetworkCallback calls connectivityManager registerDefaultNetworkCallback`() { - val buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - val registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - contextMock, - logger, - buildInfo, - mock(), - ) - - assertTrue(registered) - verify(connectivityManager).registerDefaultNetworkCallback(any()) - } - @Test fun `unregisterNetworkCallback calls connectivityManager unregisterDefaultNetworkCallback`() { whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) @@ -631,4 +609,27 @@ class AndroidConnectionStatusProviderTest { // changed significantly) verify(observer, times(2)).onConnectionStatusChanged(any()) } + + @Test + fun `childCallbacks receive network events dispatched by provider`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Register a mock child callback + val childCallback = mock() + AndroidConnectionStatusProvider.getChildCallbacks().add(childCallback) + + // Simulate event on available + mainCallback.onAvailable(network) + + // Assert child callback received the event + verify(childCallback).onAvailable(network) + + // Remove it and ensure it no longer receives events + AndroidConnectionStatusProvider.getChildCallbacks().remove(childCallback) + mainCallback.onAvailable(network) + verifyNoMoreInteractions(childCallback) + } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt index 90d830eee47..2ae12c03c3a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt @@ -54,7 +54,15 @@ internal interface CaptureStrategy { fun convert(): CaptureStrategy companion object { - private const val BREADCRUMB_START_OFFSET = 100L + private fun Breadcrumb?.isNetworkAvailable(): Boolean = + this != null && + category == "network.event" && + data.getOrElse("action", { null }) == "NETWORK_AVAILABLE" + + private fun Breadcrumb.isNetworkConnectivity(): Boolean = + category == "network.event" && data.containsKey("network_type") + + private const val NETWORK_BREADCRUMB_START_OFFSET = 5000L // 5 minutes, otherwise relay will just drop it. Can prevent the case where the device // time is wrong and the segment is too long. @@ -168,12 +176,18 @@ internal interface CaptureStrategy { } val urls = LinkedList() + var previousCrumb: Breadcrumb? = null breadcrumbs.forEach { breadcrumb -> - // we add some fixed breadcrumb offset to make sure we don't miss any - // breadcrumbs that might be relevant for the current segment, but just happened - // earlier than the current segment (e.g. network connectivity changed) + // we special-case network-reconnected breadcrumb, because there's usually some delay after + // we receive onConnected callback and we resume ongoing replay recording. We still want + // this breadcrumb to be sent with the current segment, hence we give it more room to make + // it into the replay + val isAfterNetworkReconnected = + previousCrumb?.isNetworkAvailable() == true && + breadcrumb.isNetworkConnectivity() && + breadcrumb.timestamp.time + NETWORK_BREADCRUMB_START_OFFSET >= segmentTimestamp.time if ( - (breadcrumb.timestamp.time + BREADCRUMB_START_OFFSET) >= segmentTimestamp.time && + (breadcrumb.timestamp.time >= segmentTimestamp.time || isAfterNetworkReconnected) && breadcrumb.timestamp.time < endTimestamp.time ) { val rrwebEvent = options.replayController.breadcrumbConverter.convert(breadcrumb) @@ -190,6 +204,7 @@ internal interface CaptureStrategy { } } } + previousCrumb = breadcrumb } if (screenAtStart != null && urls.firstOrNull() != screenAtStart) { diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 37ba75783a4..b08e140392c 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -97,7 +97,9 @@ public void close() throws IOException { @Override public void onConnectionStatusChanged( final @NotNull IConnectionStatusProvider.ConnectionStatus status) { - if (scopes != null && options != null) { + if (scopes != null + && options != null + && status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) { sendCachedEnvelopes(scopes, options); } } From 83d80d7bafd9e1aa7ce6a3f703f6c81608db7810 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 18 Jul 2025 16:32:35 +0200 Subject: [PATCH 10/43] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2a6fbf4978..9832c9cb7db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) - Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) +- Have single `NetworkCallback` registered at a time to reduce IPC calls ([#4562](https://github.com/getsentry/sentry-java/pull/4562)) ## 8.17.0 From 5b3266256330f8da1aca82b707fab065e5193d9f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 23 Jul 2025 13:42:05 +0200 Subject: [PATCH 11/43] perf(integrations): Use single lifecycle observer --- .../core/AndroidOptionsInitializer.java | 1 + .../android/core/AppLifecycleIntegration.java | 97 +++------- .../java/io/sentry/android/core/AppState.java | 169 +++++++++++++++++- .../sentry/android/core/LifecycleWatcher.java | 14 +- .../SystemEventsBreadcrumbsIntegration.java | 147 ++++----------- .../SystemEventsBreadcrumbsIntegrationTest.kt | 86 +++------ 6 files changed, 251 insertions(+), 263 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index e4874b4309b..71050022f6f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -128,6 +128,7 @@ static void loadDefaultAndMetadataOptions( options.setCacheDirPath(getCacheDir(context).getAbsolutePath()); readDefaultOptionValues(options, context, buildInfoProvider); + AppState.getInstance().addLifecycleObserver(options); } @TestOnly diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java index 92bf2203481..44a1ae1a5bd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java @@ -4,10 +4,12 @@ import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.IScopes; +import io.sentry.ISentryLifecycleToken; import io.sentry.Integration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.AndroidThreadChecker; +import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; @@ -17,20 +19,11 @@ public final class AppLifecycleIntegration implements Integration, Closeable { + private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); @TestOnly @Nullable volatile LifecycleWatcher watcher; private @Nullable SentryAndroidOptions options; - private final @NotNull MainLooperHandler handler; - - public AppLifecycleIntegration() { - this(new MainLooperHandler()); - } - - AppLifecycleIntegration(final @NotNull MainLooperHandler handler) { - this.handler = handler; - } - @Override public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions options) { Objects.requireNonNull(scopes, "Scopes are required"); @@ -55,85 +48,47 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions if (this.options.isEnableAutoSessionTracking() || this.options.isEnableAppLifecycleBreadcrumbs()) { - try { - Class.forName("androidx.lifecycle.DefaultLifecycleObserver"); - Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); - if (AndroidThreadChecker.getInstance().isMainThread()) { - addObserver(scopes); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - handler.post(() -> addObserver(scopes)); + try (final ISentryLifecycleToken ignored = lock.acquire()) { + if (watcher != null) { + return; } - } catch (ClassNotFoundException e) { - options - .getLogger() - .log( - SentryLevel.WARNING, - "androidx.lifecycle is not available, AppLifecycleIntegration won't be installed"); - } catch (IllegalStateException e) { - options - .getLogger() - .log(SentryLevel.ERROR, "AppLifecycleIntegration could not be installed", e); - } - } - } - private void addObserver(final @NotNull IScopes scopes) { - // this should never happen, check added to avoid warnings from NullAway - if (this.options == null) { - return; - } + watcher = + new LifecycleWatcher( + scopes, + this.options.getSessionTrackingIntervalMillis(), + this.options.isEnableAutoSessionTracking(), + this.options.isEnableAppLifecycleBreadcrumbs()); - watcher = - new LifecycleWatcher( - scopes, - this.options.getSessionTrackingIntervalMillis(), - this.options.isEnableAutoSessionTracking(), - this.options.isEnableAppLifecycleBreadcrumbs()); + AppState.getInstance().addAppStateListener(watcher); + } - try { - ProcessLifecycleOwner.get().getLifecycle().addObserver(watcher); options.getLogger().log(SentryLevel.DEBUG, "AppLifecycleIntegration installed."); addIntegrationToSdkVersion("AppLifecycle"); - } catch (Throwable e) { - // This is to handle a potential 'AbstractMethodError' gracefully. The error is triggered in - // connection with conflicting dependencies of the androidx.lifecycle. - // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 - watcher = null; - options - .getLogger() - .log( - SentryLevel.ERROR, - "AppLifecycleIntegration failed to get Lifecycle and could not be installed.", - e); } } private void removeObserver() { - final @Nullable LifecycleWatcher watcherRef = watcher; + final @Nullable LifecycleWatcher watcherRef; + try (final ISentryLifecycleToken ignored = lock.acquire()) { + watcherRef = watcher; + watcher = null; + } + if (watcherRef != null) { - ProcessLifecycleOwner.get().getLifecycle().removeObserver(watcherRef); + AppState.getInstance().removeAppStateListener(watcherRef); if (options != null) { options.getLogger().log(SentryLevel.DEBUG, "AppLifecycleIntegration removed."); } } - watcher = null; } @Override public void close() throws IOException { - if (watcher == null) { - return; - } - if (AndroidThreadChecker.getInstance().isMainThread()) { - removeObserver(); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - // avoid method refs on Android due to some issues with older AGP setups - // noinspection Convert2MethodRef - handler.post(() -> removeObserver()); - } + removeObserver(); + // TODO: probably should move it to Scopes.close(), but that'd require a new interface and + // different implementations for Java and Android. This is probably fine like this too, because + // integrations are closed in the same place + AppState.getInstance().removeLifecycleObserver(); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index d9633aed540..e15a69257cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -1,7 +1,20 @@ package io.sentry.android.core; +import androidx.annotation.NonNull; +import androidx.lifecycle.DefaultLifecycleObserver; +import androidx.lifecycle.Lifecycle; +import androidx.lifecycle.LifecycleOwner; +import androidx.lifecycle.ProcessLifecycleOwner; +import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; +import io.sentry.NoOpLogger; +import io.sentry.SentryLevel; +import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.util.AutoClosableReentrantLock; +import java.io.Closeable; +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -9,9 +22,11 @@ /** AppState holds the state of the App, e.g. whether the app is in background/foreground, etc. */ @ApiStatus.Internal -public final class AppState { +public final class AppState implements Closeable { private static @NotNull AppState instance = new AppState(); private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); + volatile LifecycleObserver lifecycleObserver; + MainLooperHandler handler = new MainLooperHandler(); private AppState() {} @@ -19,7 +34,7 @@ private AppState() {} return instance; } - private @Nullable Boolean inBackground = null; + private volatile @Nullable Boolean inBackground = null; @TestOnly void resetInstance() { @@ -31,8 +46,156 @@ void resetInstance() { } void setInBackground(final boolean inBackground) { + this.inBackground = inBackground; + } + + void addAppStateListener(final @NotNull AppStateListener listener) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + ensureLifecycleObserver(NoOpLogger.getInstance()); + + lifecycleObserver.listeners.add(listener); + } + } + + void removeAppStateListener(final @NotNull AppStateListener listener) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (lifecycleObserver != null) { + lifecycleObserver.listeners.remove(listener); + } + } + } + + void addLifecycleObserver(final @Nullable SentryAndroidOptions options) { + if (lifecycleObserver != null) { + return; + } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - this.inBackground = inBackground; + ensureLifecycleObserver(options != null ? options.getLogger() : NoOpLogger.getInstance()); + } + } + + private void ensureLifecycleObserver(final @NotNull ILogger logger) { + if (lifecycleObserver != null) { + return; } + try { + Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); + // create it right away, so it's available in addAppStateListener in case it's posted to main thread + lifecycleObserver = new LifecycleObserver(); + + if (AndroidThreadChecker.getInstance().isMainThread()) { + addObserverInternal(logger); + } else { + // some versions of the androidx lifecycle-process require this to be executed on the main + // thread. + handler.post(() -> addObserverInternal(logger)); + } + } catch (ClassNotFoundException e) { + logger + .log( + SentryLevel.WARNING, + "androidx.lifecycle is not available, some features might not be properly working," + + "e.g. Session Tracking, Network and System Events breadcrumbs, etc."); + } catch (Throwable e) { + logger + .log( + SentryLevel.ERROR, + "AppState could not register lifecycle observer", + e); + } + } + + private void addObserverInternal(final @NotNull ILogger logger) { + final @Nullable LifecycleObserver observerRef = lifecycleObserver; + try { + // might already be unregistered/removed so we have to check for nullability + if (observerRef != null) { + ProcessLifecycleOwner.get().getLifecycle().addObserver(observerRef); + } + } catch (Throwable e) { + // This is to handle a potential 'AbstractMethodError' gracefully. The error is triggered in + // connection with conflicting dependencies of the androidx.lifecycle. + // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 + lifecycleObserver = null; + logger + .log( + SentryLevel.ERROR, + "AppState failed to get Lifecycle and could not install lifecycle observer.", + e); + } + } + + void removeLifecycleObserver() { + if (lifecycleObserver == null) { + return; + } + + final @Nullable LifecycleObserver ref; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + ref = lifecycleObserver; + lifecycleObserver.listeners.clear(); + lifecycleObserver = null; + } + + if (AndroidThreadChecker.getInstance().isMainThread()) { + removeObserverInternal(ref); + } else { + // some versions of the androidx lifecycle-process require this to be executed on the main + // thread. + // avoid method refs on Android due to some issues with older AGP setups + // noinspection Convert2MethodRef + handler.post(() -> removeObserverInternal(ref)); + } + } + + private void removeObserverInternal(final @Nullable LifecycleObserver ref) { + if (ref != null) { + ProcessLifecycleOwner.get().getLifecycle().removeObserver(ref); + } + } + + @Override + public void close() throws IOException { + removeLifecycleObserver(); + } + + static final class LifecycleObserver implements DefaultLifecycleObserver { + final List listeners = new CopyOnWriteArrayList() { + @Override + public boolean add(AppStateListener appStateListener) { + // notify the listeners immediately to let them "catch up" with the current state (mimics the behavior of androidx.lifecycle) + Lifecycle.State currentState = ProcessLifecycleOwner.get().getLifecycle().getCurrentState(); + if (currentState.isAtLeast(Lifecycle.State.STARTED)) { + appStateListener.onForeground(); + } else { + appStateListener.onBackground(); + } + return super.add(appStateListener); + } + }; + + @Override + public void onStart(@NonNull LifecycleOwner owner) { + for (AppStateListener listener : listeners) { + listener.onForeground(); + } + AppState.getInstance().setInBackground(false); + } + + @Override + public void onStop(@NonNull LifecycleOwner owner) { + for (AppStateListener listener : listeners) { + listener.onBackground(); + } + AppState.getInstance().setInBackground(true); + } + } + + // If necessary, we can adjust this and add other callbacks in the future + public interface AppStateListener { + void onForeground(); + + void onBackground(); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index 89d78193207..a5e4d398e74 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -1,7 +1,5 @@ package io.sentry.android.core; -import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.LifecycleOwner; import io.sentry.Breadcrumb; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; @@ -17,7 +15,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -final class LifecycleWatcher implements DefaultLifecycleObserver { +final class LifecycleWatcher implements AppState.AppStateListener { private final AtomicLong lastUpdatedSession = new AtomicLong(0L); @@ -58,15 +56,10 @@ final class LifecycleWatcher implements DefaultLifecycleObserver { this.currentDateProvider = currentDateProvider; } - // App goes to foreground @Override - public void onStart(final @NotNull LifecycleOwner owner) { + public void onForeground() { startSession(); addAppBreadcrumb("foreground"); - - // Consider using owner.getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED); - // in the future. - AppState.getInstance().setInBackground(false); } private void startSession() { @@ -99,14 +92,13 @@ private void startSession() { // App went to background and triggered this callback after 700ms // as no new screen was shown @Override - public void onStop(final @NotNull LifecycleOwner owner) { + public void onBackground() { final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis(); this.lastUpdatedSession.set(currentTimeMillis); scopes.getOptions().getReplayController().pause(); scheduleEndSession(); - AppState.getInstance().setInBackground(true); addAppBreadcrumb("background"); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index d681f08429a..3ab4bb4e6c3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -25,9 +25,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; -import androidx.annotation.NonNull; -import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.Breadcrumb; import io.sentry.Hint; @@ -52,16 +49,13 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -public final class SystemEventsBreadcrumbsIntegration implements Integration, Closeable { +public final class SystemEventsBreadcrumbsIntegration implements Integration, Closeable, + AppState.AppStateListener { private final @NotNull Context context; @TestOnly @Nullable volatile SystemEventsBroadcastReceiver receiver; - @TestOnly @Nullable volatile ReceiverLifecycleHandler lifecycleHandler; - - private final @NotNull MainLooperHandler handler; - private @Nullable SentryAndroidOptions options; private @Nullable IScopes scopes; @@ -76,18 +70,10 @@ public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) { this(context, getDefaultActionsInternal()); } - private SystemEventsBreadcrumbsIntegration( - final @NotNull Context context, final @NotNull String[] actions) { - this(context, actions, new MainLooperHandler()); - } - SystemEventsBreadcrumbsIntegration( - final @NotNull Context context, - final @NotNull String[] actions, - final @NotNull MainLooperHandler handler) { + final @NotNull Context context, final @NotNull String[] actions) { this.context = ContextUtils.getApplicationContext(context); this.actions = actions; - this.handler = handler; } public SystemEventsBreadcrumbsIntegration( @@ -95,7 +81,6 @@ public SystemEventsBreadcrumbsIntegration( this.context = ContextUtils.getApplicationContext(context); this.actions = new String[actions.size()]; actions.toArray(this.actions); - this.handler = new MainLooperHandler(); } @Override @@ -115,7 +100,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions this.options.isEnableSystemEventBreadcrumbs()); if (this.options.isEnableSystemEventBreadcrumbs()) { - addLifecycleObserver(this.options); + AppState.getInstance().addAppStateListener(this); registerReceiver(this.scopes, this.options, /* reportAsNewIntegration= */ true); } } @@ -129,10 +114,8 @@ private void registerReceiver( return; } - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - if (isClosed || isStopped || receiver != null) { - return; - } + if (isClosed || isStopped || receiver != null) { + return; } try { @@ -183,88 +166,22 @@ private void registerReceiver( } private void unregisterReceiver() { - final @Nullable SystemEventsBroadcastReceiver receiverRef; - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - isStopped = true; - receiverRef = receiver; - receiver = null; - } - - if (receiverRef != null) { - context.unregisterReceiver(receiverRef); + if (options == null) { + return; } - } - // TODO: this duplicates a lot of AppLifecycleIntegration. We should register once on init - // and multiplex to different listeners rather. - private void addLifecycleObserver(final @NotNull SentryAndroidOptions options) { - try { - Class.forName("androidx.lifecycle.DefaultLifecycleObserver"); - Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); - if (AndroidThreadChecker.getInstance().isMainThread()) { - addObserverInternal(options); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - handler.post(() -> addObserverInternal(options)); + options.getExecutorService().submit(() -> { + final @Nullable SystemEventsBroadcastReceiver receiverRef; + try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { + isStopped = true; + receiverRef = receiver; + receiver = null; } - } catch (ClassNotFoundException e) { - options - .getLogger() - .log( - SentryLevel.WARNING, - "androidx.lifecycle is not available, SystemEventsBreadcrumbsIntegration won't be able" - + " to register/unregister an internal BroadcastReceiver. This may result in an" - + " increased ANR rate on Android 14 and above."); - } catch (Throwable e) { - options - .getLogger() - .log( - SentryLevel.ERROR, - "SystemEventsBreadcrumbsIntegration could not register lifecycle observer", - e); - } - } - private void addObserverInternal(final @NotNull SentryAndroidOptions options) { - lifecycleHandler = new ReceiverLifecycleHandler(); - - try { - ProcessLifecycleOwner.get().getLifecycle().addObserver(lifecycleHandler); - } catch (Throwable e) { - // This is to handle a potential 'AbstractMethodError' gracefully. The error is triggered in - // connection with conflicting dependencies of the androidx.lifecycle. - // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 - lifecycleHandler = null; - options - .getLogger() - .log( - SentryLevel.ERROR, - "SystemEventsBreadcrumbsIntegration failed to get Lifecycle and could not install lifecycle observer.", - e); - } - } - - private void removeLifecycleObserver() { - if (lifecycleHandler != null) { - if (AndroidThreadChecker.getInstance().isMainThread()) { - removeObserverInternal(); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - // avoid method refs on Android due to some issues with older AGP setups - // noinspection Convert2MethodRef - handler.post(() -> removeObserverInternal()); + if (receiverRef != null) { + context.unregisterReceiver(receiverRef); } - } - } - - private void removeObserverInternal() { - final @Nullable ReceiverLifecycleHandler watcherRef = lifecycleHandler; - if (watcherRef != null) { - ProcessLifecycleOwner.get().getLifecycle().removeObserver(watcherRef); - } - lifecycleHandler = null; + }); } @Override @@ -274,11 +191,11 @@ public void close() throws IOException { filter = null; } - removeLifecycleObserver(); + AppState.getInstance().removeAppStateListener(this); unregisterReceiver(); if (options != null) { - options.getLogger().log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration remove."); + options.getLogger().log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration removed."); } } @@ -311,24 +228,20 @@ public void close() throws IOException { return actions; } - final class ReceiverLifecycleHandler implements DefaultLifecycleObserver { - @Override - public void onStart(@NonNull LifecycleOwner owner) { - if (scopes == null || options == null) { - return; - } + @Override + public void onForeground() { + if (scopes == null || options == null) { + return; + } - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - isStopped = false; - } + isStopped = false; - registerReceiver(scopes, options, /* reportAsNewIntegration= */ false); - } + registerReceiver(scopes, options, /* reportAsNewIntegration= */ false); + } - @Override - public void onStop(@NonNull LifecycleOwner owner) { - unregisterReceiver(); - } + @Override + public void onBackground() { + unregisterReceiver(); } static final class SystemEventsBroadcastReceiver extends BroadcastReceiver { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index d35df7c8444..0784621c28a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -30,6 +30,8 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config +import kotlin.test.BeforeTest +import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -38,14 +40,11 @@ class SystemEventsBreadcrumbsIntegrationTest { val context = mock() var options = SentryAndroidOptions() val scopes = mock() - lateinit var handler: MainLooperHandler fun getSut( enableSystemEventBreadcrumbs: Boolean = true, executorService: ISentryExecutorService = ImmediateExecutorService(), - mockHandler: Boolean = true, ): SystemEventsBreadcrumbsIntegration { - handler = if (mockHandler) mock() else MainLooperHandler() options = SentryAndroidOptions().apply { isEnableSystemEventBreadcrumbs = enableSystemEventBreadcrumbs @@ -54,13 +53,17 @@ class SystemEventsBreadcrumbsIntegrationTest { return SystemEventsBreadcrumbsIntegration( context, SystemEventsBreadcrumbsIntegration.getDefaultActions().toTypedArray(), - handler, ) } } private val fixture = Fixture() + @BeforeTest + fun `set up`() { + AppState.getInstance().resetInstance() + } + @Test fun `When system events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() @@ -337,82 +340,44 @@ class SystemEventsBreadcrumbsIntegrationTest { } @Test - fun `When integration is added, lifecycle handler should be started`() { + fun `When integration is added, should subscribe for app state events`() { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.lifecycleHandler) + assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test - fun `When system events breadcrumbs are disabled, lifecycle handler should not be started`() { + fun `When system events breadcrumbs are disabled, should not subscribe for app state events`() { val sut = fixture.getSut() fixture.options.apply { isEnableSystemEventBreadcrumbs = false } sut.register(fixture.scopes, fixture.options) - assertNull(sut.lifecycleHandler) + assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test - fun `When integration is closed, lifecycle handler should be closed`() { + fun `When integration is closed, should unsubscribe from app state events`() { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.lifecycleHandler) + assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) sut.close() - assertNull(sut.lifecycleHandler) + assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test - fun `When integration is registered from a background thread, post on the main thread`() { - val sut = fixture.getSut() - val latch = CountDownLatch(1) - - Thread { - sut.register(fixture.scopes, fixture.options) - latch.countDown() - } - .start() - - latch.await() - - verify(fixture.handler).post(any()) - } - - @Test - fun `When integration is closed from a background thread, post on the main thread`() { + fun `When integration is closed from a background thread, unsubscribes from app events`() { val sut = fixture.getSut() val latch = CountDownLatch(1) sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.lifecycleHandler) - - Thread { - sut.close() - latch.countDown() - } - .start() - - latch.await() - - verify(fixture.handler).post(any()) - } - - @Test - fun `When integration is closed from a background thread, watcher is set to null`() { - val sut = fixture.getSut(mockHandler = false) - val latch = CountDownLatch(1) - - sut.register(fixture.scopes, fixture.options) - - assertNotNull(sut.lifecycleHandler) - Thread { sut.close() latch.countDown() @@ -424,7 +389,7 @@ class SystemEventsBreadcrumbsIntegrationTest { // ensure all messages on main looper got processed shadowOf(Looper.getMainLooper()).idle() - assertNull(sut.lifecycleHandler) + assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test @@ -433,7 +398,7 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - sut.lifecycleHandler!!.onStop(mock()) + sut.onBackground() verify(fixture.context).unregisterReceiver(any()) assertNull(sut.receiver) @@ -446,8 +411,8 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) verify(fixture.context).registerReceiver(any(), any(), any()) - sut.lifecycleHandler!!.onStop(mock()) - sut.lifecycleHandler!!.onStart(mock()) + sut.onBackground() + sut.onForeground() verify(fixture.context, times(2)).registerReceiver(any(), any(), any()) assertNotNull(sut.receiver) @@ -461,7 +426,7 @@ class SystemEventsBreadcrumbsIntegrationTest { verify(fixture.context).registerReceiver(any(), any(), any()) val receiver = sut.receiver - sut.lifecycleHandler!!.onStart(mock()) + sut.onForeground() assertEquals(receiver, sut.receiver) } @@ -473,10 +438,10 @@ class SystemEventsBreadcrumbsIntegrationTest { deferredExecutorService.runAll() assertNotNull(sut.receiver) - sut.lifecycleHandler!!.onStop(mock()) - sut.lifecycleHandler!!.onStart(mock()) + sut.onBackground() + sut.onForeground() assertNull(sut.receiver) - sut.lifecycleHandler!!.onStop(mock()) + sut.onBackground() deferredExecutorService.runAll() assertNull(sut.receiver) } @@ -486,7 +451,7 @@ class SystemEventsBreadcrumbsIntegrationTest { val deferredExecutorService = DeferredExecutorService() val latch = CountDownLatch(1) - val sut = fixture.getSut(executorService = deferredExecutorService, mockHandler = false) + val sut = fixture.getSut(executorService = deferredExecutorService) sut.register(fixture.scopes, fixture.options) deferredExecutorService.runAll() assertNotNull(sut.receiver) @@ -499,13 +464,12 @@ class SystemEventsBreadcrumbsIntegrationTest { latch.await() - sut.lifecycleHandler!!.onStart(mock()) + sut.onForeground() assertNull(sut.receiver) deferredExecutorService.runAll() shadowOf(Looper.getMainLooper()).idle() assertNull(sut.receiver) - assertNull(sut.lifecycleHandler) } } From 5c5238a9a27c61d531eb165839a1d7b09af32945 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 24 Jul 2025 12:41:36 +0200 Subject: [PATCH 12/43] Add tests --- .../api/sentry-android-core.api | 12 +- .../core/AndroidOptionsInitializer.java | 2 +- .../android/core/AppLifecycleIntegration.java | 4 +- .../java/io/sentry/android/core/AppState.java | 67 ++--- .../SystemEventsBreadcrumbsIntegration.java | 31 +- .../core/AppLifecycleIntegrationTest.kt | 54 +--- .../io/sentry/android/core/AppStateTest.kt | 283 ++++++++++++++++++ .../SystemEventsBreadcrumbsIntegrationTest.kt | 43 ++- 8 files changed, 393 insertions(+), 103 deletions(-) create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 987f13f540f..c3c71aa75f4 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -166,11 +166,17 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/AppState { +public final class io/sentry/android/core/AppState : java/io/Closeable { + public fun close ()V public static fun getInstance ()Lio/sentry/android/core/AppState; public fun isInBackground ()Ljava/lang/Boolean; } +public abstract interface class io/sentry/android/core/AppState$AppStateListener { + public abstract fun onBackground ()V + public abstract fun onForeground ()V +} + public final class io/sentry/android/core/BuildConfig { public static final field BUILD_TYPE Ljava/lang/String; public static final field DEBUG Z @@ -420,11 +426,13 @@ public class io/sentry/android/core/SpanFrameMetricsCollector : io/sentry/IPerfo public fun onSpanStarted (Lio/sentry/ISpan;)V } -public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { +public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, io/sentry/android/core/AppState$AppStateListener, java/io/Closeable { public fun (Landroid/content/Context;)V public fun (Landroid/content/Context;Ljava/util/List;)V public fun close ()V public static fun getDefaultActions ()Ljava/util/List; + public fun onBackground ()V + public fun onForeground ()V public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 71050022f6f..40a0727a817 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -128,7 +128,7 @@ static void loadDefaultAndMetadataOptions( options.setCacheDirPath(getCacheDir(context).getAbsolutePath()); readDefaultOptionValues(options, context, buildInfoProvider); - AppState.getInstance().addLifecycleObserver(options); + AppState.getInstance().registerLifecycleObserver(options); } @TestOnly diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java index 44a1ae1a5bd..9fd90b23099 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java @@ -2,13 +2,11 @@ import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion; -import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; import io.sentry.Integration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; -import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; import java.io.Closeable; @@ -89,6 +87,6 @@ public void close() throws IOException { // TODO: probably should move it to Scopes.close(), but that'd require a new interface and // different implementations for Java and Android. This is probably fine like this too, because // integrations are closed in the same place - AppState.getInstance().removeLifecycleObserver(); + AppState.getInstance().unregisterLifecycleObserver(); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index e15a69257cd..855385631d0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -2,7 +2,6 @@ import androidx.annotation.NonNull; import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.Lifecycle; import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.ILogger; @@ -65,7 +64,7 @@ void removeAppStateListener(final @NotNull AppStateListener listener) { } } - void addLifecycleObserver(final @Nullable SentryAndroidOptions options) { + void registerLifecycleObserver(final @Nullable SentryAndroidOptions options) { if (lifecycleObserver != null) { return; } @@ -81,7 +80,8 @@ private void ensureLifecycleObserver(final @NotNull ILogger logger) { } try { Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); - // create it right away, so it's available in addAppStateListener in case it's posted to main thread + // create it right away, so it's available in addAppStateListener in case it's posted to main + // thread lifecycleObserver = new LifecycleObserver(); if (AndroidThreadChecker.getInstance().isMainThread()) { @@ -92,17 +92,12 @@ private void ensureLifecycleObserver(final @NotNull ILogger logger) { handler.post(() -> addObserverInternal(logger)); } } catch (ClassNotFoundException e) { - logger - .log( - SentryLevel.WARNING, - "androidx.lifecycle is not available, some features might not be properly working," - + "e.g. Session Tracking, Network and System Events breadcrumbs, etc."); + logger.log( + SentryLevel.WARNING, + "androidx.lifecycle is not available, some features might not be properly working," + + "e.g. Session Tracking, Network and System Events breadcrumbs, etc."); } catch (Throwable e) { - logger - .log( - SentryLevel.ERROR, - "AppState could not register lifecycle observer", - e); + logger.log(SentryLevel.ERROR, "AppState could not register lifecycle observer", e); } } @@ -118,15 +113,14 @@ private void addObserverInternal(final @NotNull ILogger logger) { // connection with conflicting dependencies of the androidx.lifecycle. // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 lifecycleObserver = null; - logger - .log( - SentryLevel.ERROR, - "AppState failed to get Lifecycle and could not install lifecycle observer.", - e); + logger.log( + SentryLevel.ERROR, + "AppState failed to get Lifecycle and could not install lifecycle observer.", + e); } } - void removeLifecycleObserver() { + void unregisterLifecycleObserver() { if (lifecycleObserver == null) { return; } @@ -157,30 +151,31 @@ private void removeObserverInternal(final @Nullable LifecycleObserver ref) { @Override public void close() throws IOException { - removeLifecycleObserver(); + unregisterLifecycleObserver(); } - static final class LifecycleObserver implements DefaultLifecycleObserver { - final List listeners = new CopyOnWriteArrayList() { - @Override - public boolean add(AppStateListener appStateListener) { - // notify the listeners immediately to let them "catch up" with the current state (mimics the behavior of androidx.lifecycle) - Lifecycle.State currentState = ProcessLifecycleOwner.get().getLifecycle().getCurrentState(); - if (currentState.isAtLeast(Lifecycle.State.STARTED)) { - appStateListener.onForeground(); - } else { - appStateListener.onBackground(); - } - return super.add(appStateListener); - } - }; + final class LifecycleObserver implements DefaultLifecycleObserver { + final List listeners = + new CopyOnWriteArrayList() { + @Override + public boolean add(AppStateListener appStateListener) { + // notify the listeners immediately to let them "catch up" with the current state + // (mimics the behavior of androidx.lifecycle) + if (Boolean.FALSE.equals(inBackground)) { + appStateListener.onForeground(); + } else if (Boolean.TRUE.equals(inBackground)) { + appStateListener.onBackground(); + } + return super.add(appStateListener); + } + }; @Override public void onStart(@NonNull LifecycleOwner owner) { for (AppStateListener listener : listeners) { listener.onForeground(); } - AppState.getInstance().setInBackground(false); + setInBackground(false); } @Override @@ -188,7 +183,7 @@ public void onStop(@NonNull LifecycleOwner owner) { for (AppStateListener listener : listeners) { listener.onBackground(); } - AppState.getInstance().setInBackground(true); + setInBackground(true); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 3ab4bb4e6c3..802d40d3d28 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -25,7 +25,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; -import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.Breadcrumb; import io.sentry.Hint; import io.sentry.IScopes; @@ -34,7 +33,6 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; -import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.internal.util.Debouncer; import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; @@ -49,8 +47,8 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -public final class SystemEventsBreadcrumbsIntegration implements Integration, Closeable, - AppState.AppStateListener { +public final class SystemEventsBreadcrumbsIntegration + implements Integration, Closeable, AppState.AppStateListener { private final @NotNull Context context; @@ -170,18 +168,21 @@ private void unregisterReceiver() { return; } - options.getExecutorService().submit(() -> { - final @Nullable SystemEventsBroadcastReceiver receiverRef; - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - isStopped = true; - receiverRef = receiver; - receiver = null; - } + options + .getExecutorService() + .submit( + () -> { + final @Nullable SystemEventsBroadcastReceiver receiverRef; + try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { + isStopped = true; + receiverRef = receiver; + receiver = null; + } - if (receiverRef != null) { - context.unregisterReceiver(receiverRef); - } - }); + if (receiverRef != null) { + context.unregisterReceiver(receiverRef); + } + }); } @Override diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt index 6b2cafabe7f..896673085c2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt @@ -8,21 +8,17 @@ import kotlin.test.Test import kotlin.test.assertNotNull import kotlin.test.assertNull import org.junit.runner.RunWith -import org.mockito.kotlin.any import org.mockito.kotlin.mock -import org.mockito.kotlin.verify import org.robolectric.Shadows.shadowOf @RunWith(AndroidJUnit4::class) class AppLifecycleIntegrationTest { private class Fixture { val scopes = mock() - lateinit var handler: MainLooperHandler val options = SentryAndroidOptions() - fun getSut(mockHandler: Boolean = true): AppLifecycleIntegration { - handler = if (mockHandler) mock() else MainLooperHandler() - return AppLifecycleIntegration(handler) + fun getSut(): AppLifecycleIntegration { + return AppLifecycleIntegration() } } @@ -64,23 +60,7 @@ class AppLifecycleIntegrationTest { } @Test - fun `When AppLifecycleIntegration is registered from a background thread, post on the main thread`() { - val sut = fixture.getSut() - val latch = CountDownLatch(1) - - Thread { - sut.register(fixture.scopes, fixture.options) - latch.countDown() - } - .start() - - latch.await() - - verify(fixture.handler).post(any()) - } - - @Test - fun `When AppLifecycleIntegration is closed from a background thread, post on the main thread`() { + fun `When AppLifecycleIntegration is closed from a background thread, watcher is set to null`() { val sut = fixture.getSut() val latch = CountDownLatch(1) @@ -96,29 +76,25 @@ class AppLifecycleIntegrationTest { latch.await() - verify(fixture.handler).post(any()) + // ensure all messages on main looper got processed + shadowOf(Looper.getMainLooper()).idle() + + assertNull(sut.watcher) } @Test - fun `When AppLifecycleIntegration is closed from a background thread, watcher is set to null`() { - val sut = fixture.getSut(mockHandler = false) - val latch = CountDownLatch(1) + fun `When AppLifecycleIntegration is closed, AppState unregisterLifecycleObserver is called`() { + val sut = fixture.getSut() + val appState = AppState.getInstance() sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.watcher) + // Verify that lifecycleObserver is not null after registration + assertNotNull(appState.lifecycleObserver) - Thread { - sut.close() - latch.countDown() - } - .start() - - latch.await() - - // ensure all messages on main looper got processed - shadowOf(Looper.getMainLooper()).idle() + sut.close() - assertNull(sut.watcher) + // Verify that lifecycleObserver is null after unregistering + assertNull(appState.lifecycleObserver) } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt new file mode 100644 index 00000000000..efef483daf4 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt @@ -0,0 +1,283 @@ +package io.sentry.android.core + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.android.core.AppState.AppStateListener +import io.sentry.android.core.internal.util.AndroidThreadChecker +import java.util.concurrent.CountDownLatch +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertSame +import kotlin.test.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +@RunWith(AndroidJUnit4::class) +class AppStateTest { + + private class Fixture { + val mockThreadChecker: AndroidThreadChecker = mock() + val mockHandler: MainLooperHandler = mock() + val options = SentryAndroidOptions() + val listener: AppStateListener = mock() + lateinit var androidThreadCheckerMock: MockedStatic + + fun getSut(isMainThread: Boolean = true): AppState { + val appState = AppState.getInstance() + whenever(mockThreadChecker.isMainThread).thenReturn(isMainThread) + appState.handler = mockHandler + return appState + } + + fun createListener(): AppStateListener = mock() + } + + private val fixture = Fixture() + + @BeforeTest + fun `set up`() { + AppState.getInstance().resetInstance() + + // Mock AndroidThreadChecker + fixture.androidThreadCheckerMock = mockStatic(AndroidThreadChecker::class.java) + fixture.androidThreadCheckerMock + .`when` { AndroidThreadChecker.getInstance() } + .thenReturn(fixture.mockThreadChecker) + } + + @AfterTest + fun `tear down`() { + fixture.androidThreadCheckerMock.close() + } + + @Test + fun `getInstance returns singleton instance`() { + val instance1 = fixture.getSut() + val instance2 = fixture.getSut() + + assertSame(instance1, instance2) + } + + @Test + fun `resetInstance creates new instance`() { + val sut = fixture.getSut() + sut.setInBackground(true) + + sut.resetInstance() + + val newInstance = fixture.getSut() + assertNull(newInstance.isInBackground()) + } + + @Test + fun `isInBackground returns null initially`() { + val sut = fixture.getSut() + + assertNull(sut.isInBackground()) + } + + @Test + fun `setInBackground updates state`() { + val sut = fixture.getSut() + + sut.setInBackground(true) + assertTrue(sut.isInBackground()!!) + + sut.setInBackground(false) + assertFalse(sut.isInBackground()!!) + } + + @Test + fun `addAppStateListener creates lifecycle observer if needed`() { + val sut = fixture.getSut() + + sut.addAppStateListener(fixture.listener) + + assertNotNull(sut.lifecycleObserver) + } + + @Test + fun `addAppStateListener from background thread posts to main thread`() { + val sut = fixture.getSut(isMainThread = false) + + sut.addAppStateListener(fixture.listener) + + verify(fixture.mockHandler).post(any()) + } + + @Test + fun `addAppStateListener notifies listener with onForeground when in foreground state`() { + val sut = fixture.getSut() + + sut.setInBackground(false) + sut.addAppStateListener(fixture.listener) + + verify(fixture.listener).onForeground() + verify(fixture.listener, never()).onBackground() + } + + @Test + fun `addAppStateListener notifies listener with onBackground when in background state`() { + val sut = fixture.getSut() + + sut.setInBackground(true) + sut.addAppStateListener(fixture.listener) + + verify(fixture.listener).onBackground() + verify(fixture.listener, never()).onForeground() + } + + @Test + fun `addAppStateListener does not notify listener when state is unknown`() { + val sut = fixture.getSut() + + // State is null (unknown) by default + sut.addAppStateListener(fixture.listener) + + verify(fixture.listener, never()).onForeground() + verify(fixture.listener, never()).onBackground() + } + + @Test + fun `removeAppStateListener removes listener`() { + val sut = fixture.getSut() + + sut.addAppStateListener(fixture.listener) + val observer = sut.lifecycleObserver + // Check that listener was added + assertNotNull(observer) + + sut.removeAppStateListener(fixture.listener) + // Listener should be removed but observer still exists + assertNotNull(sut.lifecycleObserver) + } + + @Test + fun `removeAppStateListener handles null lifecycle observer`() { + val sut = fixture.getSut() + + // Should not throw when lifecycleObserver is null + sut.removeAppStateListener(fixture.listener) + } + + @Test + fun `registerLifecycleObserver does nothing if already registered`() { + val sut = fixture.getSut() + + sut.registerLifecycleObserver(fixture.options) + val firstObserver = sut.lifecycleObserver + + sut.registerLifecycleObserver(fixture.options) + val secondObserver = sut.lifecycleObserver + + assertSame(firstObserver, secondObserver) + } + + @Test + fun `unregisterLifecycleObserver clears listeners and nulls observer`() { + val sut = fixture.getSut() + + sut.addAppStateListener(fixture.listener) + assertNotNull(sut.lifecycleObserver) + + sut.unregisterLifecycleObserver() + + assertNull(sut.lifecycleObserver) + } + + @Test + fun `unregisterLifecycleObserver handles null observer`() { + val sut = fixture.getSut() + + // Should not throw when lifecycleObserver is already null + sut.unregisterLifecycleObserver() + } + + @Test + fun `unregisterLifecycleObserver from background thread posts to main thread`() { + val sut = fixture.getSut(isMainThread = false) + + sut.registerLifecycleObserver(fixture.options) + + sut.unregisterLifecycleObserver() + + // 2 times - register and unregister + verify(fixture.mockHandler, times(2)).post(any()) + } + + @Test + fun `close calls unregisterLifecycleObserver`() { + val sut = fixture.getSut() + sut.addAppStateListener(fixture.listener) + + sut.close() + + assertNull(sut.lifecycleObserver) + } + + @Test + fun `LifecycleObserver onStart notifies all listeners and sets foreground`() { + val listener1 = fixture.createListener() + val listener2 = fixture.createListener() + val sut = fixture.getSut() + + // Add listeners to create observer + sut.addAppStateListener(listener1) + sut.addAppStateListener(listener2) + + val observer = sut.lifecycleObserver!! + observer.onStart(mock()) + + verify(listener1).onForeground() + verify(listener2).onForeground() + assertFalse(sut.isInBackground()!!) + } + + @Test + fun `LifecycleObserver onStop notifies all listeners and sets background`() { + val listener1 = fixture.createListener() + val listener2 = fixture.createListener() + val sut = fixture.getSut() + + // Add listeners to create observer + sut.addAppStateListener(listener1) + sut.addAppStateListener(listener2) + + val observer = sut.lifecycleObserver!! + observer.onStop(mock()) + + verify(listener1).onBackground() + verify(listener2).onBackground() + assertTrue(sut.isInBackground()!!) + } + + @Test + fun `thread safety - concurrent access is handled`() { + val listeners = (1..5).map { fixture.createListener() } + val sut = fixture.getSut() + val latch = CountDownLatch(5) + + // Add listeners concurrently + listeners.forEach { listener -> + Thread { + sut.addAppStateListener(listener) + latch.countDown() + } + .start() + } + latch.await() + + val observer = sut.lifecycleObserver + assertNotNull(observer) + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 0784621c28a..7cae0d67c67 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -13,11 +13,14 @@ import io.sentry.SentryLevel import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import java.util.concurrent.CountDownLatch +import kotlin.test.AfterTest +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -30,8 +33,6 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config -import kotlin.test.BeforeTest -import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -62,6 +63,12 @@ class SystemEventsBreadcrumbsIntegrationTest { @BeforeTest fun `set up`() { AppState.getInstance().resetInstance() + AppState.getInstance().registerLifecycleObserver(fixture.options) + } + + @AfterTest + fun `tear down`() { + AppState.getInstance().unregisterLifecycleObserver() } @Test @@ -345,7 +352,11 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertTrue( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -355,7 +366,11 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -364,11 +379,19 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertTrue( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) sut.close() - assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -389,7 +412,11 @@ class SystemEventsBreadcrumbsIntegrationTest { // ensure all messages on main looper got processed shadowOf(Looper.getMainLooper()).idle() - assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -440,6 +467,7 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.onBackground() sut.onForeground() + deferredExecutorService.runAll() assertNull(sut.receiver) sut.onBackground() deferredExecutorService.runAll() @@ -463,6 +491,7 @@ class SystemEventsBreadcrumbsIntegrationTest { .start() latch.await() + deferredExecutorService.runAll() sut.onForeground() assertNull(sut.receiver) From d2263b85223404f369c40ac59144806a6ec76da4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 24 Jul 2025 12:54:58 +0200 Subject: [PATCH 13/43] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9832c9cb7db..06b9cd1e029 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Internal + +- Use single `LifecycleObserver` and multi-cast it to the integrations interested in lifecycle states ([#4567](https://github.com/getsentry/sentry-java/pull/4567)) + ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) From 42ff8e98f30fa888a0f4c87e639c2999e43d18c7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 24 Jul 2025 13:19:47 +0200 Subject: [PATCH 14/43] Fix tests --- .../android/core/LifecycleWatcherTest.kt | 64 +++++++------------ .../core/SessionTrackingIntegrationTest.kt | 11 ++-- 2 files changed, 27 insertions(+), 48 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt index 446dfa3330a..5149f167129 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt @@ -1,6 +1,5 @@ package io.sentry.android.core -import androidx.lifecycle.LifecycleOwner import io.sentry.Breadcrumb import io.sentry.DateUtils import io.sentry.IContinuousProfiler @@ -16,10 +15,8 @@ import io.sentry.transport.ICurrentDateProvider import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull -import kotlin.test.assertTrue import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.check @@ -33,7 +30,6 @@ import org.mockito.kotlin.whenever class LifecycleWatcherTest { private class Fixture { - val ownerMock = mock() val scopes = mock() val dateProvider = mock() val options = SentryOptions() @@ -77,7 +73,7 @@ class LifecycleWatcherTest { @Test fun `if last started session is 0, start new session`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes).startSession() verify(fixture.replayController).start() } @@ -86,8 +82,8 @@ class LifecycleWatcherTest { fun `if last started session is after interval, start new session`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L, 2L) - watcher.onStart(fixture.ownerMock) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() + watcher.onForeground() verify(fixture.scopes, times(2)).startSession() verify(fixture.replayController, times(2)).start() } @@ -96,8 +92,8 @@ class LifecycleWatcherTest { fun `if last started session is before interval, it should not start a new session`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) whenever(fixture.dateProvider.currentTimeMillis).thenReturn(2L, 1L) - watcher.onStart(fixture.ownerMock) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() + watcher.onForeground() verify(fixture.scopes).startSession() verify(fixture.replayController).start() } @@ -105,8 +101,8 @@ class LifecycleWatcherTest { @Test fun `if app goes to background, end session after interval`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) - watcher.onStop(fixture.ownerMock) + watcher.onForeground() + watcher.onBackground() verify(fixture.scopes, timeout(10000)).endSession() verify(fixture.replayController, timeout(10000)).stop() verify(fixture.continuousProfiler, timeout(10000)).close(eq(false)) @@ -116,12 +112,12 @@ class LifecycleWatcherTest { fun `if app goes to background and foreground again, dont end the session`() { val watcher = fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() - watcher.onStop(fixture.ownerMock) + watcher.onBackground() assertNotNull(watcher.timerTask) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() assertNull(watcher.timerTask) verify(fixture.scopes, never()).endSession() @@ -132,7 +128,7 @@ class LifecycleWatcherTest { fun `When session tracking is disabled, do not start session`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes, never()).startSession() } @@ -140,14 +136,14 @@ class LifecycleWatcherTest { fun `When session tracking is disabled, do not end session`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.scopes, never()).endSession() } @Test fun `When app lifecycle breadcrumbs is enabled, add breadcrumb on start`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes) .addBreadcrumb( check { @@ -163,14 +159,14 @@ class LifecycleWatcherTest { fun `When app lifecycle breadcrumbs is disabled, do not add breadcrumb on start`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes, never()).addBreadcrumb(any()) } @Test fun `When app lifecycle breadcrumbs is enabled, add breadcrumb on stop`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false) - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.scopes) .addBreadcrumb( check { @@ -186,7 +182,7 @@ class LifecycleWatcherTest { fun `When app lifecycle breadcrumbs is disabled, do not add breadcrumb on stop`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.scopes, never()).addBreadcrumb(any()) } @@ -221,7 +217,7 @@ class LifecycleWatcherTest { ), ) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes, never()).startSession() verify(fixture.replayController, never()).start() } @@ -250,25 +246,11 @@ class LifecycleWatcherTest { ), ) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes).startSession() verify(fixture.replayController).start() } - @Test - fun `When app goes into foreground, sets isBackground to false for AppState`() { - val watcher = fixture.getSUT() - watcher.onStart(fixture.ownerMock) - assertFalse(AppState.getInstance().isInBackground!!) - } - - @Test - fun `When app goes into background, sets isBackground to true for AppState`() { - val watcher = fixture.getSUT() - watcher.onStop(fixture.ownerMock) - assertTrue(AppState.getInstance().isInBackground!!) - } - @Test fun `if the hub has already a fresh session running, resumes replay to invalidate isManualPause flag`() { val watcher = @@ -293,7 +275,7 @@ class LifecycleWatcherTest { ), ) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.replayController).resume() } @@ -301,16 +283,16 @@ class LifecycleWatcherTest { fun `background-foreground replay`() { whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L) val watcher = fixture.getSUT(sessionIntervalMillis = 2L, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.replayController).start() - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.replayController).pause() - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.replayController, times(2)).resume() - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.replayController, timeout(10000)).stop() } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt index d4264d9831b..bdb328e2421 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt @@ -18,7 +18,6 @@ import io.sentry.SentryEnvelope import io.sentry.SentryEvent import io.sentry.SentryLogEvent import io.sentry.SentryLogEvents -import io.sentry.SentryOptions import io.sentry.SentryReplayEvent import io.sentry.Session import io.sentry.TraceContext @@ -41,6 +40,7 @@ class SessionTrackingIntegrationTest { @BeforeTest fun `set up`() { + AppState.getInstance().resetInstance() context = ApplicationProvider.getApplicationContext() } @@ -56,7 +56,7 @@ class SessionTrackingIntegrationTest { } val client = CapturingSentryClient() Sentry.bindClient(client) - val lifecycle = setupLifecycle(options) + val lifecycle = setupLifecycle() val initSid = lastSessionId() lifecycle.handleLifecycleEvent(ON_START) @@ -115,12 +115,9 @@ class SessionTrackingIntegrationTest { return sid } - private fun setupLifecycle(options: SentryOptions): LifecycleRegistry { + private fun setupLifecycle(): LifecycleRegistry { val lifecycle = LifecycleRegistry(ProcessLifecycleOwner.get()) - val lifecycleWatcher = - (options.integrations.find { it is AppLifecycleIntegration } as AppLifecycleIntegration) - .watcher - lifecycle.addObserver(lifecycleWatcher!!) + lifecycle.addObserver(AppState.getInstance().lifecycleObserver) return lifecycle } From fde11a247f10aa0219eb69b68032023cb64db9ee Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 00:45:12 +0200 Subject: [PATCH 15/43] perf(integrations): Do not register for SystemEvents and NetworkCallbacks when launched with background importance --- .../java/io/sentry/android/core/AppState.java | 7 +- .../SystemEventsBreadcrumbsIntegration.java | 15 +- .../util/AndroidConnectionStatusProvider.java | 163 +++++++++++++----- .../AndroidConnectionStatusProviderTest.kt | 6 +- .../sentry-samples-android/sdkperf/README.md | 78 +++++++++ .../sentry-samples-android/sdkperf/basic.pbtx | 103 +++++++++++ .../sdkperf/screen_flap.sh | 15 ++ .../sdkperf/wifi_flap.sh | 19 ++ .../src/main/AndroidManifest.xml | 15 +- .../sentry/samples/android/DummyService.java | 69 ++++++++ 10 files changed, 430 insertions(+), 60 deletions(-) create mode 100644 sentry-samples/sentry-samples-android/sdkperf/README.md create mode 100644 sentry-samples/sentry-samples-android/sdkperf/basic.pbtx create mode 100755 sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh create mode 100755 sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh create mode 100644 sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index 855385631d0..21da2bcc161 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -1,5 +1,6 @@ package io.sentry.android.core; +import android.util.Log; import androidx.annotation.NonNull; import androidx.lifecycle.DefaultLifecycleObserver; import androidx.lifecycle.LifecycleOwner; @@ -48,7 +49,7 @@ void setInBackground(final boolean inBackground) { this.inBackground = inBackground; } - void addAppStateListener(final @NotNull AppStateListener listener) { + public void addAppStateListener(final @NotNull AppStateListener listener) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { ensureLifecycleObserver(NoOpLogger.getInstance()); @@ -56,7 +57,7 @@ void addAppStateListener(final @NotNull AppStateListener listener) { } } - void removeAppStateListener(final @NotNull AppStateListener listener) { + public void removeAppStateListener(final @NotNull AppStateListener listener) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { if (lifecycleObserver != null) { lifecycleObserver.listeners.remove(listener); @@ -172,10 +173,10 @@ public boolean add(AppStateListener appStateListener) { @Override public void onStart(@NonNull LifecycleOwner owner) { + setInBackground(false); for (AppStateListener listener : listeners) { listener.onForeground(); } - setInBackground(false); } @Override diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 802d40d3d28..f2345fe00de 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -25,6 +25,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; +import android.util.Log; import io.sentry.Breadcrumb; import io.sentry.Hint; import io.sentry.IScopes; @@ -43,6 +44,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -62,6 +64,7 @@ public final class SystemEventsBreadcrumbsIntegration private volatile boolean isClosed = false; private volatile boolean isStopped = false; private volatile IntentFilter filter = null; + private final @NotNull AtomicBoolean isReceiverRegistered = new AtomicBoolean(false); private final @NotNull AutoClosableReentrantLock receiverLock = new AutoClosableReentrantLock(); public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) { @@ -99,14 +102,16 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions if (this.options.isEnableSystemEventBreadcrumbs()) { AppState.getInstance().addAppStateListener(this); - registerReceiver(this.scopes, this.options, /* reportAsNewIntegration= */ true); + + if (ContextUtils.isForegroundImportance()) { + registerReceiver(this.scopes, this.options); + } } } private void registerReceiver( final @NotNull IScopes scopes, - final @NotNull SentryAndroidOptions options, - final boolean reportAsNewIntegration) { + final @NotNull SentryAndroidOptions options) { if (!options.isEnableSystemEventBreadcrumbs()) { return; @@ -137,7 +142,7 @@ private void registerReceiver( // registerReceiver can throw SecurityException but it's not documented in the // official docs ContextUtils.registerReceiver(context, options, receiver, filter); - if (reportAsNewIntegration) { + if (!isReceiverRegistered.getAndSet(true)) { options .getLogger() .log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration installed."); @@ -237,7 +242,7 @@ public void onForeground() { isStopped = false; - registerReceiver(scopes, options, /* reportAsNewIntegration= */ false); + registerReceiver(scopes, options); } @Override diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 40e96eeb922..aefea29ea92 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -13,14 +13,20 @@ import io.sentry.IConnectionStatusProvider; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; +import io.sentry.Sentry; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.android.core.AppState; import io.sentry.android.core.BuildInfoProvider; import io.sentry.android.core.ContextUtils; import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -32,7 +38,8 @@ * details */ @ApiStatus.Internal -public final class AndroidConnectionStatusProvider implements IConnectionStatusProvider { +public final class AndroidConnectionStatusProvider implements IConnectionStatusProvider, + AppState.AppStateListener { private final @NotNull Context context; private final @NotNull SentryOptions options; @@ -58,12 +65,12 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP private static final int[] capabilities = new int[2]; - private final @NotNull Thread initThread; private volatile @Nullable NetworkCapabilities cachedNetworkCapabilities; private volatile @Nullable Network currentNetwork; private volatile long lastCacheUpdateTime = 0; private static final long CACHE_TTL_MS = 2 * 60 * 1000L; // 2 minutes - + private final @NotNull AtomicBoolean isConnected = new AtomicBoolean(false); + @SuppressLint("InlinedApi") public AndroidConnectionStatusProvider( @NotNull Context context, @@ -83,8 +90,9 @@ public AndroidConnectionStatusProvider( // Register network callback immediately for caching //noinspection Convert2MethodRef - initThread = new Thread(() -> ensureNetworkCallbackRegistered()); - initThread.start(); + submitSafe(() -> ensureNetworkCallbackRegistered()); + + AppState.getInstance().addAppStateListener(this); } /** @@ -148,6 +156,10 @@ private boolean isNetworkEffectivelyConnected( } private void ensureNetworkCallbackRegistered() { + if (!ContextUtils.isForegroundImportance()) { + return; + } + if (networkCallback != null) { return; // Already registered } @@ -163,9 +175,13 @@ private void ensureNetworkCallbackRegistered() { public void onAvailable(final @NotNull Network network) { currentNetwork = network; - try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { - for (final @NotNull NetworkCallback cb : childCallbacks) { - cb.onAvailable(network); + // have to only dispatch this on first registration + when the connection got re-established + // otherwise it would've been dispatched on every foreground + if (!isConnected.getAndSet(true)) { + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onAvailable(network); + } } } } @@ -197,6 +213,7 @@ public void onLost(final @NotNull Network network) { } private void clearCacheAndNotifyObservers() { + isConnected.set(false); // Clear cached capabilities and network reference atomically try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { cachedNetworkCapabilities = null; @@ -413,42 +430,84 @@ public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObser } } + private void unregisterNetworkCallback(final boolean clearObservers) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (clearObservers) { + connectionStatusObservers.clear(); + } + + final @Nullable NetworkCallback callbackRef = networkCallback; + networkCallback = null; + + if (callbackRef != null) { + unregisterNetworkCallback(context, options.getLogger(), callbackRef); + } + // Clear cached state + cachedNetworkCapabilities = null; + currentNetwork = null; + lastCacheUpdateTime = 0; + } + options.getLogger().log(SentryLevel.DEBUG, "Network callback unregistered"); + } + /** Clean up resources - should be called when the provider is no longer needed */ @Override public void close() { - try { - options - .getExecutorService() - .submit( - () -> { - final NetworkCallback callbackRef; - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - connectionStatusObservers.clear(); + submitSafe( + () -> { + unregisterNetworkCallback(/* clearObservers = */true); + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.clear(); + } + try (final @NotNull ISentryLifecycleToken ignored = + connectivityManagerLock.acquire()) { + connectivityManager = null; + } + }); + } - callbackRef = networkCallback; - networkCallback = null; + @Override + public void onForeground() { + if (networkCallback != null) { + return; + } - if (callbackRef != null) { - unregisterNetworkCallback(context, options.getLogger(), callbackRef); - } - // Clear cached state - cachedNetworkCapabilities = null; - currentNetwork = null; - lastCacheUpdateTime = 0; - } - try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { - childCallbacks.clear(); - } - try (final @NotNull ISentryLifecycleToken ignored = - connectivityManagerLock.acquire()) { - connectivityManager = null; - } - }); - } catch (Throwable t) { - options - .getLogger() - .log(SentryLevel.ERROR, "Error submitting AndroidConnectionStatusProvider task", t); + submitSafe(() -> { + // proactively update cache and notify observers on foreground to ensure connectivity state is not stale + updateCache(null); + + final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); + if (status == ConnectionStatus.DISCONNECTED) { + // onLost is not called retroactively when we registerNetworkCallback (as opposed to onAvailable), so we have to do it manually for the DISCONNECTED case + isConnected.set(false); + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + //noinspection DataFlowIssue + cb.onLost(null); + } + } + } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + for (final @NotNull IConnectionStatusObserver observer : + connectionStatusObservers) { + observer.onConnectionStatusChanged(status); + } + } + + ensureNetworkCallbackRegistered(); + }); + } + + @Override + public void onBackground() { + if (networkCallback == null) { + return; } + + submitSafe(() -> { + //noinspection Convert2MethodRef + unregisterNetworkCallback(/* clearObservers = */false); + }); } /** @@ -595,7 +654,6 @@ public NetworkCapabilities getCachedNetworkCapabilities() { if (cellular) { return "cellular"; } - } catch (Throwable exception) { logger.log(SentryLevel.ERROR, "Failed to retrieve network info", exception); } @@ -730,15 +788,30 @@ public NetworkCallback getNetworkCallback() { return networkCallback; } - @TestOnly - @NotNull - public Thread getInitThread() { - return initThread; - } - @TestOnly @NotNull public static List getChildCallbacks() { return childCallbacks; } + + private void submitSafe(@NotNull Runnable r) { + try { + options.getExecutorService().submit(r); + } catch (Throwable e) { + options.getLogger() + .log(SentryLevel.ERROR, "AndroidConnectionStatusProvider submit failed", e); + } + } + + private static class ConnectionStatusProviderThreadyFactory implements ThreadFactory { + + private int cnt = 0; + + @Override + public Thread newThread(Runnable r) { + final Thread ret = new Thread(r, "SentryConnectionStatusProvider-" + cnt++); + ret.setDaemon(true); + return ret; + } + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index 6769ac15301..3fc6af3fc3a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -31,6 +31,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.mockingDetails @@ -39,6 +40,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever +import java.util.concurrent.ExecutorService class AndroidConnectionStatusProviderTest { private lateinit var connectionStatusProvider: AndroidConnectionStatusProvider @@ -93,9 +95,6 @@ class AndroidConnectionStatusProviderTest { connectionStatusProvider = AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider) - - // Wait for async callback registration to complete - connectionStatusProvider.initThread.join() } @AfterTest @@ -164,7 +163,6 @@ class AndroidConnectionStatusProviderTest { // Create a new provider with the null connectivity manager val providerWithNullConnectivity = AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider) - providerWithNullConnectivity.initThread.join() // Wait for async init to complete assertEquals( IConnectionStatusProvider.ConnectionStatus.UNKNOWN, diff --git a/sentry-samples/sentry-samples-android/sdkperf/README.md b/sentry-samples/sentry-samples-android/sdkperf/README.md new file mode 100644 index 00000000000..401c6407b35 --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/README.md @@ -0,0 +1,78 @@ +This folder contains various artifacts and info related to testing SDK performance and behaviour under different circumstances. + +## Perfetto + +The `basic.pbtx` file contains a perfetto config which covers some basic data sources and things that you usually would be interested in while experimenting with the SDK. + +You can adjust some certain things like `duration_ms` to make the trace last longer or add additional [data sources](https://perfetto.dev/docs/data-sources/atrace). + +To run it, ensure you have a device available via `adb` and then run: + +```bash +adb shell perfetto \ + -c basic.pbtx --txt \ + -o /data/misc/perfetto-traces/trace +``` + +And then perform various activities you're interested in. After the trace has finished, you can pull it: + +```bash +adb pull /data/misc/perfetto-traces/trace +``` + +And open it up in https://ui.perfetto.dev/. + +## Network Connectivity + +Android has a weird behavior which has been fixed in [Android 15](https://cs.android.com/android/_/android/platform/packages/modules/Connectivity/+/2d78124348f4864d054ea7a7b52683d225bd7c1f), where it would queue up pending NetworkCallbacks while an app is being frozen and would deliver **all** of them after the app unfreezes in a quick succession. + +Since our SDK is listening to NetworkCallbacks in [AndroidConnectionStatusProvider](../../../sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java) to determine current network connectivity status and to create breadcrumbs, our SDK can be burst with potentially hundreds or thousands of events after hours or days of the hosting app inactivity. + +The following steps are necessary to reproduce the issue: + +1. Launch the sample app and send it to background +2. Freeze it with `adb shell am freeze --sticky io.sentry.samples.android` +3. Run the `./wifi_flap` script which looses and obtains network connectivity 10 times. +4. Unfreeze the app with `adb shell am unfreeze io.sentry.samples.android` + +You can either watch Logcat or better start a Perfetto trace beforehand and then open it and observe the number of binder calls our SDK is doing to the Connectivity service. + +### Solution + +We have addressed the issue in [link PR]() by unsubscribing from network connectivity updates when app goes to background and re-subscribing again on foreground. + +## System Events + +[Android 14](https://developer.android.com/develop/background-work/background-tasks/broadcasts#android-14) introduced a new behavior that defers any system broadcasts while an app is in a cached state. These pending broadcasts will be delivered to the app once it gets uncached in a quick succession. + +Our SDK is listening to a bunch of broadcasts in [SystemEventsBreadcrumbsIntegration](../../../sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java) to create breadcrumbs, the SDK can be burst with potentially hundreds or thousands of pending broadcasts after hours or days of the hosting app inactivity. + +The following steps are necessary to reproduce the issue: + +1. Launch the sample app and send it to background +2. Freeze it with `adb shell am freeze --sticky io.sentry.samples.android` +3. Run the `./screen_flap` script which turns the screen on and off 10 times. +4. Unfreeze the app with `adb shell am unfreeze io.sentry.samples.android` + +And watch Logcat for a bunch of `SCREEN_OFF`/`SCREEN_ON` breadcrumbs created microseconds apart. + +### Solution + +We have addressed the issue in [#4338](https://github.com/getsentry/sentry-java/pull/4338) by unregistering the `BroadcastReceiver` when app goes to background and registering it again on foreground. + +## App Launch with Background Importance + +While the above two issues can be fixed by observing the App lifecycle, they still may become a problem if the app process has been launched with non-foreground importance (e.g. received a push notification). In this case our SDK would be initialized too + +The following steps are necessary to reproduce the issue: + +1. Launch the sample app +2. Kill it with `adb shell am force-stop io.sentry.samples.android` +3. Now launch a dummy service with `adb shell am start-foreground-service -n io.sentry.samples.android/io.sentry.samples.android.DummyService`. This ensures the app process is run with non-foreground importance. +4. Follow any of the steps described in the sections above. + +Observe (Logcat or Perfetto) that the faulty behaviours are still reproducible. + +### Solution + +We have addressed the issue in [link PR]() by not registering any of the offending integrations when the hosting app process is launched with non-foreground `importance`. We still keep observing the App Lifecycle to ensure we register the integrations when the App has been brought to foreground. \ No newline at end of file diff --git a/sentry-samples/sentry-samples-android/sdkperf/basic.pbtx b/sentry-samples/sentry-samples-android/sdkperf/basic.pbtx new file mode 100644 index 00000000000..20f599b9b06 --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/basic.pbtx @@ -0,0 +1,103 @@ +buffers { + size_kb: 265536 + fill_policy: DISCARD +} +buffers { + size_kb: 4096 + fill_policy: DISCARD +} +data_sources { + config { + name: "linux.ftrace" + ftrace_config { + ftrace_events: "sched/sched_process_exit" + ftrace_events: "sched/sched_process_free" + ftrace_events: "task/task_newtask" + ftrace_events: "task/task_rename" + ftrace_events: "sched/sched_switch" + ftrace_events: "power/suspend_resume" + ftrace_events: "sched/sched_blocked_reason" + ftrace_events: "sched/sched_wakeup" + ftrace_events: "sched/sched_wakeup_new" + ftrace_events: "sched/sched_waking" + ftrace_events: "sched/sched_process_exit" + ftrace_events: "sched/sched_process_free" + ftrace_events: "task/task_newtask" + ftrace_events: "task/task_rename" + ftrace_events: "power/cpu_frequency" + ftrace_events: "power/cpu_idle" + ftrace_events: "power/suspend_resume" + ftrace_events: "power/gpu_frequency" + ftrace_events: "power/gpu_work_period" + ftrace_events: "ftrace/print" + atrace_categories: "adb" + atrace_categories: "camera" + atrace_categories: "gfx" + atrace_categories: "network" + atrace_categories: "power" + atrace_categories: "wm" + atrace_categories: "am" + atrace_categories: "dalvik" + atrace_categories: "bionic" + atrace_categories: "binder_driver" + atrace_categories: "binder_lock" + atrace_categories: "ss" + atrace_apps: "*" + + symbolize_ksyms: true + } + } +} +data_sources { + config { + name: "linux.process_stats" + process_stats_config { + scan_all_processes_on_start: true + proc_stats_poll_ms: 250 + } + } +} +data_sources { + config { + name: "linux.sys_stats" + } +} +data_sources { + config { + name: "android.log" + android_log_config { + } + } +} +data_sources { + config { + name: "android.surfaceflinger.frametimeline" + } +} +data_sources { + config { + name: "linux.perf" + perf_event_config { + remote_descriptor_timeout_ms: 10000 + ring_buffer_pages: 8192 + + timebase { + period: 1 + + tracepoint { + #name: "binder/binder_transaction" + name: "binder/binder_transaction_received" + } + } + callstack_sampling { + scope { + target_cmdline: "io.sentry.samples.android" + } + kernel_frames: true + } + } + } +} + + +duration_ms: 100000 \ No newline at end of file diff --git a/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh b/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh new file mode 100755 index 00000000000..2059d0d120d --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +# Loop 4 times to toggle Wi-Fi +for i in {1..4} +do + echo "[$i] Turning screen off..." + adb shell input keyevent 223 + sleep 5 + + echo "[$i] Turning screen on..." + adb shell input keyevent 224 + sleep 5 +done + +echo "Done flapping Screen 4 times." diff --git a/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh b/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh new file mode 100755 index 00000000000..8d45718e620 --- /dev/null +++ b/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh @@ -0,0 +1,19 @@ +#!/bin/bash + +# Disable mobile data first +echo "Disabling mobile data..." +adb shell svc data disable + +# Loop 8 times to toggle Wi-Fi +for i in {1..8} +do + echo "[$i] Disabling Wi-Fi..." + adb shell svc wifi disable + sleep 2 + + echo "[$i] Enabling Wi-Fi..." + adb shell svc wifi enable + sleep 6 +done + +echo "Done flapping Wi-Fi 8 times." diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index d26b2a485e4..abbbf6940c8 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -12,7 +12,10 @@ - + + + + @@ -29,6 +32,12 @@ android:networkSecurityConfig="@xml/network" tools:ignore="GoogleAppIndexingWarning, UnusedAttribute"> + + @@ -116,7 +125,7 @@ - + @@ -169,7 +178,7 @@ - + diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java new file mode 100644 index 00000000000..ed3b062aaaa --- /dev/null +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java @@ -0,0 +1,69 @@ +package io.sentry.samples.android; + +import android.app.Notification; +import android.app.NotificationChannel; +import android.app.NotificationManager; +import android.app.Service; +import android.content.Intent; +import android.os.Build; +import android.os.IBinder; +import android.util.Log; + +public class DummyService extends Service { + + private static final String TAG = "DummyService"; + private static final String CHANNEL_ID = "dummy_service_channel"; + + @Override + public void onCreate() { + super.onCreate(); + Log.d(TAG, "DummyService created"); + createNotificationChannel(); + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + Log.d(TAG, "DummyService started"); + + Notification notification = null; + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + notification = new Notification.Builder(this, CHANNEL_ID) + .setContentTitle("Dummy Service Running") + .setContentText("Used for background broadcast testing.") + .setSmallIcon(android.R.drawable.ic_menu_info_details) + .build(); + } + + if (notification != null) { + startForeground(1, notification); + } + + // You can stop immediately or keep running + // stopSelf(); + + return START_NOT_STICKY; + } + + @Override + public void onDestroy() { + super.onDestroy(); + Log.d(TAG, "DummyService destroyed"); + } + + @Override + public IBinder onBind(Intent intent) { + return null; + } + + private void createNotificationChannel() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + NotificationChannel channel = new NotificationChannel( + CHANNEL_ID, + "Dummy Service Channel", + NotificationManager.IMPORTANCE_LOW + ); + NotificationManager manager = getSystemService(NotificationManager.class); + manager.createNotificationChannel(channel); + } + } +} From bba4efec6d05cdf32b6b07fb1741f57b925eb784 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 12:27:34 +0200 Subject: [PATCH 16/43] Do not cache importance --- .../io/sentry/android/core/ContextUtils.java | 25 +++++++------------ .../util/AndroidConnectionStatusProvider.java | 3 --- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java index 0f1c63ef278..80138000cff 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java @@ -91,20 +91,6 @@ private ContextUtils() {} // to avoid doing a bunch of Binder calls we use LazyEvaluator to cache the values that are static // during the app process running - private static final @NotNull LazyEvaluator isForegroundImportance = - new LazyEvaluator<>( - () -> { - try { - final ActivityManager.RunningAppProcessInfo appProcessInfo = - new ActivityManager.RunningAppProcessInfo(); - ActivityManager.getMyMemoryState(appProcessInfo); - return appProcessInfo.importance == IMPORTANCE_FOREGROUND; - } catch (Throwable ignored) { - // should never happen - } - return false; - }); - /** * Since this packageInfo uses flags 0 we can assume it's static and cache it as the package name * or version code cannot change during runtime, only after app update (which will spin up a new @@ -284,7 +270,15 @@ static String getVersionName(final @NotNull PackageInfo packageInfo) { */ @ApiStatus.Internal public static boolean isForegroundImportance() { - return isForegroundImportance.getValue(); + try { + final ActivityManager.RunningAppProcessInfo appProcessInfo = + new ActivityManager.RunningAppProcessInfo(); + ActivityManager.getMyMemoryState(appProcessInfo); + return appProcessInfo.importance == IMPORTANCE_FOREGROUND; + } catch (Throwable ignored) { + // should never happen + } + return false; } /** @@ -544,7 +538,6 @@ public static Context getApplicationContext(final @NotNull Context context) { @TestOnly static void resetInstance() { - isForegroundImportance.resetValue(); staticPackageInfo33.resetValue(); staticPackageInfo.resetValue(); applicationName.resetValue(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index aefea29ea92..1af76a27359 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -13,7 +13,6 @@ import io.sentry.IConnectionStatusProvider; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; -import io.sentry.Sentry; import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.AppState; @@ -23,8 +22,6 @@ import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; From 5edb0d3ff174e5e12b6f39866b0be69894650286 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 12:28:05 +0200 Subject: [PATCH 17/43] Revert SR --- .../sentry-samples-android/src/main/AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index abbbf6940c8..059a6a385d1 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -178,7 +178,7 @@ - + From d8402b6fa2f0b75526303f7f2f9b4a38a766c2fb Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 14:00:05 +0200 Subject: [PATCH 18/43] Add tests --- .../java/io/sentry/android/core/AppState.java | 25 +- .../util/AndroidConnectionStatusProvider.java | 1 + .../SystemEventsBreadcrumbsIntegrationTest.kt | 22 ++ .../AndroidConnectionStatusProviderTest.kt | 218 +++++++++++++++++- 4 files changed, 261 insertions(+), 5 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index 21da2bcc161..c79a6203e1a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -9,6 +9,7 @@ import io.sentry.ISentryLifecycleToken; import io.sentry.NoOpLogger; import io.sentry.SentryLevel; +import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.util.AutoClosableReentrantLock; import java.io.Closeable; @@ -36,11 +37,18 @@ private AppState() {} private volatile @Nullable Boolean inBackground = null; + @ApiStatus.Internal @TestOnly - void resetInstance() { + public void resetInstance() { instance = new AppState(); } + @ApiStatus.Internal + @TestOnly + public LifecycleObserver getLifecycleObserver() { + return lifecycleObserver; + } + public @Nullable Boolean isInBackground() { return inBackground; } @@ -65,7 +73,8 @@ public void removeAppStateListener(final @NotNull AppStateListener listener) { } } - void registerLifecycleObserver(final @Nullable SentryAndroidOptions options) { + @ApiStatus.Internal + public void registerLifecycleObserver(final @Nullable SentryOptions options) { if (lifecycleObserver != null) { return; } @@ -121,7 +130,8 @@ private void addObserverInternal(final @NotNull ILogger logger) { } } - void unregisterLifecycleObserver() { + @ApiStatus.Internal + public void unregisterLifecycleObserver() { if (lifecycleObserver == null) { return; } @@ -155,7 +165,8 @@ public void close() throws IOException { unregisterLifecycleObserver(); } - final class LifecycleObserver implements DefaultLifecycleObserver { + @ApiStatus.Internal + public final class LifecycleObserver implements DefaultLifecycleObserver { final List listeners = new CopyOnWriteArrayList() { @Override @@ -171,6 +182,12 @@ public boolean add(AppStateListener appStateListener) { } }; + @ApiStatus.Internal + @TestOnly + public List getListeners() { + return listeners; + } + @Override public void onStart(@NonNull LifecycleOwner owner) { setInBackground(false); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 1af76a27359..a62e35a4ece 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -460,6 +460,7 @@ public void close() { connectivityManagerLock.acquire()) { connectivityManager = null; } + AppState.getInstance().removeAppStateListener(this); }); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 7cae0d67c67..3de92aa8365 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -1,10 +1,13 @@ package io.sentry.android.core +import android.app.ActivityManager +import android.app.ActivityManager.RunningAppProcessInfo import android.content.Context import android.content.Intent import android.os.BatteryManager import android.os.Build import android.os.Looper +import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.Breadcrumb import io.sentry.IScopes @@ -33,6 +36,9 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config +import org.robolectric.shadow.api.Shadow +import org.robolectric.shadows.ShadowActivityManager +import org.robolectric.shadows.ShadowBuild @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -41,6 +47,7 @@ class SystemEventsBreadcrumbsIntegrationTest { val context = mock() var options = SentryAndroidOptions() val scopes = mock() + lateinit var shadowActivityManager: ShadowActivityManager fun getSut( enableSystemEventBreadcrumbs: Boolean = true, @@ -64,6 +71,9 @@ class SystemEventsBreadcrumbsIntegrationTest { fun `set up`() { AppState.getInstance().resetInstance() AppState.getInstance().registerLifecycleObserver(fixture.options) + ShadowBuild.reset() + val activityManager = ApplicationProvider.getApplicationContext().getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager? + fixture.shadowActivityManager = Shadow.extract(activityManager) } @AfterTest @@ -501,4 +511,16 @@ class SystemEventsBreadcrumbsIntegrationTest { assertNull(sut.receiver) } + + @Test + fun `when integration is registered in background, receiver is registered`() { + val process = RunningAppProcessInfo().apply { this.importance = RunningAppProcessInfo.IMPORTANCE_CACHED } + val processes = mutableListOf(process) + fixture.shadowActivityManager.setProcesses(processes) + + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + assertNull(sut.receiver) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index 3fc6af3fc3a..278e9e21a8a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -15,12 +15,17 @@ import android.net.NetworkCapabilities.TRANSPORT_ETHERNET import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.net.NetworkInfo import android.os.Build +import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.IConnectionStatusProvider import io.sentry.ILogger import io.sentry.SentryOptions +import io.sentry.android.core.AppState import io.sentry.android.core.BuildInfoProvider +import io.sentry.android.core.ContextUtils +import io.sentry.android.core.SystemEventsBreadcrumbsIntegration import io.sentry.test.ImmediateExecutorService import io.sentry.transport.ICurrentDateProvider +import org.junit.runner.RunWith import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -29,10 +34,15 @@ import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.clearInvocations import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq +import org.mockito.kotlin.isNull import org.mockito.kotlin.mock import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.times @@ -40,8 +50,11 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever +import org.robolectric.annotation.Config import java.util.concurrent.ExecutorService +@RunWith(AndroidJUnit4::class) +@Config(sdk = [Build.VERSION_CODES.P]) class AndroidConnectionStatusProviderTest { private lateinit var connectionStatusProvider: AndroidConnectionStatusProvider private lateinit var contextMock: Context @@ -53,6 +66,7 @@ class AndroidConnectionStatusProviderTest { private lateinit var network: Network private lateinit var networkCapabilities: NetworkCapabilities private lateinit var logger: ILogger + private lateinit var contextUtilsStaticMock: MockedStatic private var currentTime = 1000L @@ -93,6 +107,16 @@ class AndroidConnectionStatusProviderTest { // Reset current time for each test to ensure cache isolation currentTime = 1000L + // Mock ContextUtils to return foreground importance + contextUtilsStaticMock = mockStatic(ContextUtils::class.java) + contextUtilsStaticMock.`when` { ContextUtils.isForegroundImportance() } + .thenReturn(true) + contextUtilsStaticMock.`when` { ContextUtils.getApplicationContext(any()) } + .thenReturn(contextMock) + + AppState.getInstance().resetInstance() + AppState.getInstance().registerLifecycleObserver(options) + connectionStatusProvider = AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider) } @@ -101,6 +125,8 @@ class AndroidConnectionStatusProviderTest { fun `tear down`() { // clear the cache and ensure proper cleanup connectionStatusProvider.close() + contextUtilsStaticMock.close() + AppState.getInstance().unregisterLifecycleObserver() } @Test @@ -160,6 +186,16 @@ class AndroidConnectionStatusProviderTest { ) .thenReturn(PERMISSION_GRANTED) + // Need to mock ContextUtils for the new provider as well + contextUtilsStaticMock.`when` { + ContextUtils.getApplicationContext( + eq( + nullConnectivityContext + ) + ) + } + .thenReturn(nullConnectivityContext) + // Create a new provider with the null connectivity manager val providerWithNullConnectivity = AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider) @@ -441,7 +477,7 @@ class AndroidConnectionStatusProviderTest { verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) val callback = callbackCaptor.firstValue - // Set current network + // IMPORTANT: Set network as current first callback.onAvailable(network) // Lose the network @@ -630,4 +666,184 @@ class AndroidConnectionStatusProviderTest { mainCallback.onAvailable(network) verifyNoMoreInteractions(childCallback) } + + @Test + fun `onForeground notifies child callbacks when disconnected`() { + val childCallback = mock() + AndroidConnectionStatusProvider.addNetworkCallback( + contextMock, + logger, + buildInfo, + childCallback + ) + connectionStatusProvider.onBackground() + + // Setup disconnected state + whenever(connectivityManager.activeNetwork).thenReturn(null) + + connectionStatusProvider.onForeground() + + // Verify child callback was notified of lost connection with any network parameter + verify(childCallback).onLost(anyOrNull()) + } + + @Test + fun `close removes AppState listener`() { + // Clear any setup interactions + clearInvocations(connectivityManager) + + // Close the provider + connectionStatusProvider.close() + + // Now test that after closing, the provider no longer responds to lifecycle events + connectionStatusProvider.onForeground() + connectionStatusProvider.onBackground() + + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) + } + + @Test + fun `network callbacks work correctly across foreground background transitions`() { + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Get the registered callback + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // Simulate network available + callback.onAvailable(network) + + // Go to background + connectionStatusProvider.onBackground() + + // Clear the mock to reset interaction count + clearInvocations(connectivityManager) + + // Go back to foreground + connectionStatusProvider.onForeground() + + // Verify callback was re-registered + verify(connectivityManager).registerDefaultNetworkCallback(any()) + + // Verify we can still receive network events + val newCallbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(newCallbackCaptor.capture()) + val newCallback = newCallbackCaptor.lastValue + + // Simulate network capabilities change + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + + // First make the network available to set it as current + newCallback.onAvailable(network) + + // Then change capabilities + newCallback.onCapabilitiesChanged(network, newCaps) + + // Verify observer was notified (once for onForeground status update, once for capabilities change) + verify(observer, times(2)).onConnectionStatusChanged(any()) + } + + @Test + fun `onForeground registers network callback if not already registered`() { + // First ensure the network callback is not registered (simulate background state) + connectionStatusProvider.onBackground() + + // Verify callback was unregistered + assertNull(connectionStatusProvider.networkCallback) + + // Call onForeground + connectionStatusProvider.onForeground() + + // Verify network callback was registered + assertNotNull(connectionStatusProvider.networkCallback) + verify(connectivityManager, times(2)).registerDefaultNetworkCallback(any()) + } + + @Test + fun `onForeground updates cache and notifies observers`() { + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Simulate going to background first + connectionStatusProvider.onBackground() + + // Reset mock to clear previous interactions + whenever(connectivityManager.activeNetwork).thenReturn(network) + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(networkCapabilities) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(networkCapabilities.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + + // Call onForeground + connectionStatusProvider.onForeground() + + // Verify observer was notified with current status + verify(observer).onConnectionStatusChanged(IConnectionStatusProvider.ConnectionStatus.CONNECTED) + } + + @Test + fun `onForeground does nothing if callback already registered`() { + // Ensure callback is already registered + assertNotNull(connectionStatusProvider.networkCallback) + val initialCallback = connectionStatusProvider.networkCallback + + // Call onForeground + connectionStatusProvider.onForeground() + + // Verify callback hasn't changed + assertEquals(initialCallback, connectionStatusProvider.networkCallback) + // Verify registerDefaultNetworkCallback was only called once (during construction) + verify(connectivityManager, times(1)).registerDefaultNetworkCallback(any()) + } + + @Test + fun `onBackground unregisters network callback`() { + // Ensure callback is registered + assertNotNull(connectionStatusProvider.networkCallback) + + // Call onBackground + connectionStatusProvider.onBackground() + + // Verify callback was unregistered + assertNull(connectionStatusProvider.networkCallback) + verify(connectivityManager).unregisterNetworkCallback(any()) + } + + @Test + fun `onBackground does not clear observers`() { + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Call onBackground + connectionStatusProvider.onBackground() + + // Verify observer is still registered + assertEquals(1, connectionStatusProvider.statusObservers.size) + assertTrue(connectionStatusProvider.statusObservers.contains(observer)) + } + + @Test + fun `onBackground does nothing if callback not registered`() { + // First unregister by going to background + connectionStatusProvider.onBackground() + assertNull(connectionStatusProvider.networkCallback) + + // Reset mock to clear previous interactions + clearInvocations(connectivityManager) + + // Call onBackground again + connectionStatusProvider.onBackground() + + // Verify no additional unregister calls + verifyNoInteractions(connectivityManager) + } } From b7a5e0ad25257f3224a5e358d1bcb0ddde52955f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 14:01:50 +0200 Subject: [PATCH 19/43] Spotless --- .../api/sentry-android-core.api | 13 ++++ .../java/io/sentry/android/core/AppState.java | 1 - .../io/sentry/android/core/ContextUtils.java | 1 - .../SystemEventsBreadcrumbsIntegration.java | 4 +- .../util/AndroidConnectionStatusProvider.java | 72 ++++++++++--------- .../SystemEventsBreadcrumbsIntegrationTest.kt | 7 +- .../AndroidConnectionStatusProviderTest.kt | 25 +++---- .../sentry/samples/android/DummyService.java | 19 +++-- 8 files changed, 76 insertions(+), 66 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index c3c71aa75f4..604a3cd009a 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -167,9 +167,15 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In } public final class io/sentry/android/core/AppState : java/io/Closeable { + public fun addAppStateListener (Lio/sentry/android/core/AppState$AppStateListener;)V public fun close ()V public static fun getInstance ()Lio/sentry/android/core/AppState; + public fun getLifecycleObserver ()Lio/sentry/android/core/AppState$LifecycleObserver; public fun isInBackground ()Ljava/lang/Boolean; + public fun registerLifecycleObserver (Lio/sentry/SentryOptions;)V + public fun removeAppStateListener (Lio/sentry/android/core/AppState$AppStateListener;)V + public fun resetInstance ()V + public fun unregisterLifecycleObserver ()V } public abstract interface class io/sentry/android/core/AppState$AppStateListener { @@ -177,6 +183,13 @@ public abstract interface class io/sentry/android/core/AppState$AppStateListener public abstract fun onForeground ()V } +public final class io/sentry/android/core/AppState$LifecycleObserver : androidx/lifecycle/DefaultLifecycleObserver { + public fun (Lio/sentry/android/core/AppState;)V + public fun getListeners ()Ljava/util/List; + public fun onStart (Landroidx/lifecycle/LifecycleOwner;)V + public fun onStop (Landroidx/lifecycle/LifecycleOwner;)V +} + public final class io/sentry/android/core/BuildConfig { public static final field BUILD_TYPE Ljava/lang/String; public static final field DEBUG Z diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index c79a6203e1a..705c2d5eadd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -1,6 +1,5 @@ package io.sentry.android.core; -import android.util.Log; import androidx.annotation.NonNull; import androidx.lifecycle.DefaultLifecycleObserver; import androidx.lifecycle.LifecycleOwner; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java index 80138000cff..8ba0e16b7c6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java @@ -21,7 +21,6 @@ import io.sentry.SentryOptions; import io.sentry.android.core.util.AndroidLazyEvaluator; import io.sentry.protocol.App; -import io.sentry.util.LazyEvaluator; import java.io.BufferedReader; import java.io.File; import java.io.FileReader; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index f2345fe00de..a06a6adeb2e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -25,7 +25,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; -import android.util.Log; import io.sentry.Breadcrumb; import io.sentry.Hint; import io.sentry.IScopes; @@ -110,8 +109,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions } private void registerReceiver( - final @NotNull IScopes scopes, - final @NotNull SentryAndroidOptions options) { + final @NotNull IScopes scopes, final @NotNull SentryAndroidOptions options) { if (!options.isEnableSystemEventBreadcrumbs()) { return; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index a62e35a4ece..ae618bd4124 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -35,8 +35,8 @@ * details */ @ApiStatus.Internal -public final class AndroidConnectionStatusProvider implements IConnectionStatusProvider, - AppState.AppStateListener { +public final class AndroidConnectionStatusProvider + implements IConnectionStatusProvider, AppState.AppStateListener { private final @NotNull Context context; private final @NotNull SentryOptions options; @@ -67,7 +67,7 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP private volatile long lastCacheUpdateTime = 0; private static final long CACHE_TTL_MS = 2 * 60 * 1000L; // 2 minutes private final @NotNull AtomicBoolean isConnected = new AtomicBoolean(false); - + @SuppressLint("InlinedApi") public AndroidConnectionStatusProvider( @NotNull Context context, @@ -172,7 +172,8 @@ private void ensureNetworkCallbackRegistered() { public void onAvailable(final @NotNull Network network) { currentNetwork = network; - // have to only dispatch this on first registration + when the connection got re-established + // have to only dispatch this on first registration + when the connection got + // re-established // otherwise it would've been dispatched on every foreground if (!isConnected.getAndSet(true)) { try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { @@ -452,12 +453,11 @@ private void unregisterNetworkCallback(final boolean clearObservers) { public void close() { submitSafe( () -> { - unregisterNetworkCallback(/* clearObservers = */true); + unregisterNetworkCallback(/* clearObservers= */ true); try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { childCallbacks.clear(); } - try (final @NotNull ISentryLifecycleToken ignored = - connectivityManagerLock.acquire()) { + try (final @NotNull ISentryLifecycleToken ignored = connectivityManagerLock.acquire()) { connectivityManager = null; } AppState.getInstance().removeAppStateListener(this); @@ -470,30 +470,32 @@ public void onForeground() { return; } - submitSafe(() -> { - // proactively update cache and notify observers on foreground to ensure connectivity state is not stale - updateCache(null); - - final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); - if (status == ConnectionStatus.DISCONNECTED) { - // onLost is not called retroactively when we registerNetworkCallback (as opposed to onAvailable), so we have to do it manually for the DISCONNECTED case - isConnected.set(false); - try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { - for (final @NotNull NetworkCallback cb : childCallbacks) { - //noinspection DataFlowIssue - cb.onLost(null); + submitSafe( + () -> { + // proactively update cache and notify observers on foreground to ensure connectivity + // state is not stale + updateCache(null); + + final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); + if (status == ConnectionStatus.DISCONNECTED) { + // onLost is not called retroactively when we registerNetworkCallback (as opposed to + // onAvailable), so we have to do it manually for the DISCONNECTED case + isConnected.set(false); + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + //noinspection DataFlowIssue + cb.onLost(null); + } + } + } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + for (final @NotNull IConnectionStatusObserver observer : connectionStatusObservers) { + observer.onConnectionStatusChanged(status); + } } - } - } - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - for (final @NotNull IConnectionStatusObserver observer : - connectionStatusObservers) { - observer.onConnectionStatusChanged(status); - } - } - ensureNetworkCallbackRegistered(); - }); + ensureNetworkCallbackRegistered(); + }); } @Override @@ -502,10 +504,11 @@ public void onBackground() { return; } - submitSafe(() -> { - //noinspection Convert2MethodRef - unregisterNetworkCallback(/* clearObservers = */false); - }); + submitSafe( + () -> { + //noinspection Convert2MethodRef + unregisterNetworkCallback(/* clearObservers= */ false); + }); } /** @@ -796,7 +799,8 @@ private void submitSafe(@NotNull Runnable r) { try { options.getExecutorService().submit(r); } catch (Throwable e) { - options.getLogger() + options + .getLogger() .log(SentryLevel.ERROR, "AndroidConnectionStatusProvider submit failed", e); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 3de92aa8365..1cb721cdfd5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -72,7 +72,9 @@ class SystemEventsBreadcrumbsIntegrationTest { AppState.getInstance().resetInstance() AppState.getInstance().registerLifecycleObserver(fixture.options) ShadowBuild.reset() - val activityManager = ApplicationProvider.getApplicationContext().getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager? + val activityManager = + ApplicationProvider.getApplicationContext() + .getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager? fixture.shadowActivityManager = Shadow.extract(activityManager) } @@ -514,7 +516,8 @@ class SystemEventsBreadcrumbsIntegrationTest { @Test fun `when integration is registered in background, receiver is registered`() { - val process = RunningAppProcessInfo().apply { this.importance = RunningAppProcessInfo.IMPORTANCE_CACHED } + val process = + RunningAppProcessInfo().apply { this.importance = RunningAppProcessInfo.IMPORTANCE_CACHED } val processes = mutableListOf(process) fixture.shadowActivityManager.setProcesses(processes) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index 278e9e21a8a..7d27984a599 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -25,7 +25,6 @@ import io.sentry.android.core.ContextUtils import io.sentry.android.core.SystemEventsBreadcrumbsIntegration import io.sentry.test.ImmediateExecutorService import io.sentry.transport.ICurrentDateProvider -import org.junit.runner.RunWith import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -34,15 +33,14 @@ import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue +import org.junit.runner.RunWith import org.mockito.MockedStatic import org.mockito.Mockito.mockStatic import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.clearInvocations -import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq -import org.mockito.kotlin.isNull import org.mockito.kotlin.mock import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.times @@ -51,7 +49,6 @@ import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.annotation.Config -import java.util.concurrent.ExecutorService @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.P]) @@ -109,9 +106,11 @@ class AndroidConnectionStatusProviderTest { // Mock ContextUtils to return foreground importance contextUtilsStaticMock = mockStatic(ContextUtils::class.java) - contextUtilsStaticMock.`when` { ContextUtils.isForegroundImportance() } + contextUtilsStaticMock + .`when` { ContextUtils.isForegroundImportance() } .thenReturn(true) - contextUtilsStaticMock.`when` { ContextUtils.getApplicationContext(any()) } + contextUtilsStaticMock + .`when` { ContextUtils.getApplicationContext(any()) } .thenReturn(contextMock) AppState.getInstance().resetInstance() @@ -187,13 +186,8 @@ class AndroidConnectionStatusProviderTest { .thenReturn(PERMISSION_GRANTED) // Need to mock ContextUtils for the new provider as well - contextUtilsStaticMock.`when` { - ContextUtils.getApplicationContext( - eq( - nullConnectivityContext - ) - ) - } + contextUtilsStaticMock + .`when` { ContextUtils.getApplicationContext(eq(nullConnectivityContext)) } .thenReturn(nullConnectivityContext) // Create a new provider with the null connectivity manager @@ -674,7 +668,7 @@ class AndroidConnectionStatusProviderTest { contextMock, logger, buildInfo, - childCallback + childCallback, ) connectionStatusProvider.onBackground() @@ -748,7 +742,8 @@ class AndroidConnectionStatusProviderTest { // Then change capabilities newCallback.onCapabilitiesChanged(network, newCaps) - // Verify observer was notified (once for onForeground status update, once for capabilities change) + // Verify observer was notified (once for onForeground status update, once for capabilities + // change) verify(observer, times(2)).onConnectionStatusChanged(any()) } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java index ed3b062aaaa..b1d936e0cc6 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/DummyService.java @@ -27,11 +27,12 @@ public int onStartCommand(Intent intent, int flags, int startId) { Notification notification = null; if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - notification = new Notification.Builder(this, CHANNEL_ID) - .setContentTitle("Dummy Service Running") - .setContentText("Used for background broadcast testing.") - .setSmallIcon(android.R.drawable.ic_menu_info_details) - .build(); + notification = + new Notification.Builder(this, CHANNEL_ID) + .setContentTitle("Dummy Service Running") + .setContentText("Used for background broadcast testing.") + .setSmallIcon(android.R.drawable.ic_menu_info_details) + .build(); } if (notification != null) { @@ -57,11 +58,9 @@ public IBinder onBind(Intent intent) { private void createNotificationChannel() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - NotificationChannel channel = new NotificationChannel( - CHANNEL_ID, - "Dummy Service Channel", - NotificationManager.IMPORTANCE_LOW - ); + NotificationChannel channel = + new NotificationChannel( + CHANNEL_ID, "Dummy Service Channel", NotificationManager.IMPORTANCE_LOW); NotificationManager manager = getSystemService(NotificationManager.class); manager.createNotificationChannel(channel); } From c7cad14a0d28075f7e28f61cf554945b56af929d Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 14:17:27 +0200 Subject: [PATCH 20/43] Comment --- .../core/internal/util/AndroidConnectionStatusProvider.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index ae618bd4124..743d43bfbd1 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -494,6 +494,9 @@ public void onForeground() { } } + // this will ONLY do the necessary parts like registerNetworkCallback and onAvailable, but + // it won't updateCache and notify observes because we just did it above and the cached + // capabilities will be the same ensureNetworkCallbackRegistered(); }); } From 8a6f145b04652ac4a1849055c95d654fe87a8983 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 15:21:34 +0200 Subject: [PATCH 21/43] Revert profiling --- .../sentry-samples-android/src/main/AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index 059a6a385d1..9d084ed97e1 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -125,7 +125,7 @@ - + From d868d1acaa58fbd4a42a6d91806292b2c52ca368 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 15:23:18 +0200 Subject: [PATCH 22/43] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06b9cd1e029..a90ced0c451 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) - Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) - Have single `NetworkCallback` registered at a time to reduce IPC calls ([#4562](https://github.com/getsentry/sentry-java/pull/4562)) +- Do not register for SystemEvents and NetworkCallbacks immediately when launched with non-foreground importance ([#4579](https://github.com/getsentry/sentry-java/pull/4579)) ## 8.17.0 From c890fe30be07088a68b06126abc8fd11deb3eb89 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 15:26:38 +0200 Subject: [PATCH 23/43] fix test name --- .../android/core/SystemEventsBreadcrumbsIntegrationTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 1cb721cdfd5..c23130322ae 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -515,7 +515,7 @@ class SystemEventsBreadcrumbsIntegrationTest { } @Test - fun `when integration is registered in background, receiver is registered`() { + fun `when integration is registered in background, receiver is not registered`() { val process = RunningAppProcessInfo().apply { this.importance = RunningAppProcessInfo.IMPORTANCE_CACHED } val processes = mutableListOf(process) From 900ecb5039e67b1468b8bf7c89e65a6d58f2f4bb Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 15:38:51 +0200 Subject: [PATCH 24/43] Update sentry-samples/sentry-samples-android/sdkperf/README.md --- sentry-samples/sentry-samples-android/sdkperf/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/sdkperf/README.md b/sentry-samples/sentry-samples-android/sdkperf/README.md index 401c6407b35..a62a004efae 100644 --- a/sentry-samples/sentry-samples-android/sdkperf/README.md +++ b/sentry-samples/sentry-samples-android/sdkperf/README.md @@ -39,7 +39,7 @@ You can either watch Logcat or better start a Perfetto trace beforehand and then ### Solution -We have addressed the issue in [link PR]() by unsubscribing from network connectivity updates when app goes to background and re-subscribing again on foreground. +We have addressed the issue in [#4579](https://github.com/getsentry/sentry-java/pull/4579) by unsubscribing from network connectivity updates when app goes to background and re-subscribing again on foreground. ## System Events From ac613d64a21e0708c47bda78aff6834dd8697488 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 15:39:26 +0200 Subject: [PATCH 25/43] Update sentry-samples/sentry-samples-android/sdkperf/README.md --- sentry-samples/sentry-samples-android/sdkperf/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/sdkperf/README.md b/sentry-samples/sentry-samples-android/sdkperf/README.md index a62a004efae..d795733c046 100644 --- a/sentry-samples/sentry-samples-android/sdkperf/README.md +++ b/sentry-samples/sentry-samples-android/sdkperf/README.md @@ -75,4 +75,4 @@ Observe (Logcat or Perfetto) that the faulty behaviours are still reproducible. ### Solution -We have addressed the issue in [link PR]() by not registering any of the offending integrations when the hosting app process is launched with non-foreground `importance`. We still keep observing the App Lifecycle to ensure we register the integrations when the App has been brought to foreground. \ No newline at end of file +We have addressed the issue in [#4579](https://github.com/getsentry/sentry-java/pull/4579) by not registering any of the offending integrations when the hosting app process is launched with non-foreground `importance`. We still keep observing the App Lifecycle to ensure we register the integrations when the App has been brought to foreground. \ No newline at end of file From 85f5bae1520c34c25b005c6e87312e4403a89e10 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 30 Jul 2025 15:41:13 +0200 Subject: [PATCH 26/43] Update sentry-samples/sentry-samples-android/sdkperf/README.md --- sentry-samples/sentry-samples-android/sdkperf/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/sdkperf/README.md b/sentry-samples/sentry-samples-android/sdkperf/README.md index d795733c046..a350ec0d80b 100644 --- a/sentry-samples/sentry-samples-android/sdkperf/README.md +++ b/sentry-samples/sentry-samples-android/sdkperf/README.md @@ -62,7 +62,7 @@ We have addressed the issue in [#4338](https://github.com/getsentry/sentry-java/ ## App Launch with Background Importance -While the above two issues can be fixed by observing the App lifecycle, they still may become a problem if the app process has been launched with non-foreground importance (e.g. received a push notification). In this case our SDK would be initialized too +While the above two issues can be fixed by observing the App lifecycle, they still may become a problem if the app process has been launched with non-foreground importance (e.g. received a push notification). In this case our SDK would be initialized too and would still subscribe for SystemEvents and NetworkCallbacks while in background. The following steps are necessary to reproduce the issue: From 3f3a4998d0556214f87bb99145007175bd5795a3 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 1 Aug 2025 15:29:59 +0200 Subject: [PATCH 27/43] Update sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh Co-authored-by: Markus Hintersteiner --- sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh b/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh index 2059d0d120d..828d0deeef4 100755 --- a/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh +++ b/sentry-samples/sentry-samples-android/sdkperf/screen_flap.sh @@ -1,6 +1,6 @@ #!/bin/bash -# Loop 4 times to toggle Wi-Fi +# Loop 4 times to toggle Screen for i in {1..4} do echo "[$i] Turning screen off..." From c231dfcdc938c943a768a97aed0fa6dcd1352300 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 1 Aug 2025 15:30:08 +0200 Subject: [PATCH 28/43] Update sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh Co-authored-by: Markus Hintersteiner --- sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh b/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh index 8d45718e620..a468f5be477 100755 --- a/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh +++ b/sentry-samples/sentry-samples-android/sdkperf/wifi_flap.sh @@ -15,5 +15,6 @@ do adb shell svc wifi enable sleep 6 done - +# Turn mobile data back on +adb shell svc data enable echo "Done flapping Wi-Fi 8 times." From cda0b1da5bcb4ad380991e5df8c19fc7425e5728 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 1 Aug 2025 22:38:10 +0200 Subject: [PATCH 29/43] Address PR review --- .../util/AndroidConnectionStatusProvider.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 743d43bfbd1..79cdd8a432d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -807,16 +807,4 @@ private void submitSafe(@NotNull Runnable r) { .log(SentryLevel.ERROR, "AndroidConnectionStatusProvider submit failed", e); } } - - private static class ConnectionStatusProviderThreadyFactory implements ThreadFactory { - - private int cnt = 0; - - @Override - public Thread newThread(Runnable r) { - final Thread ret = new Thread(r, "SentryConnectionStatusProvider-" + cnt++); - ret.setDaemon(true); - return ret; - } - } } From f7179a5b92b4eb82286af2abf6a07813fa4b46c5 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 1 Aug 2025 22:44:57 +0200 Subject: [PATCH 30/43] Formatting --- .../core/internal/util/AndroidConnectionStatusProvider.java | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 79cdd8a432d..bb7053ab849 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -22,7 +22,6 @@ import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; From 0915d81a2ff85d649ffa21fc1ee65292e30b5a0c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 4 Aug 2025 13:42:10 +0200 Subject: [PATCH 31/43] use LazyEvaluator for empty options and timer --- .../sentry/android/core/LifecycleWatcher.java | 29 +++++++++---------- sentry/src/main/java/io/sentry/NoOpScope.java | 6 ++-- .../src/main/java/io/sentry/NoOpScopes.java | 6 ++-- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index a5e4d398e74..3d4cedb1b53 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -8,6 +8,7 @@ import io.sentry.transport.CurrentDateProvider; import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.AutoClosableReentrantLock; +import io.sentry.util.LazyEvaluator; import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.atomic.AtomicLong; @@ -22,7 +23,7 @@ final class LifecycleWatcher implements AppState.AppStateListener { private final long sessionIntervalMillis; private @Nullable TimerTask timerTask; - private final @NotNull Timer timer = new Timer(true); + private final @NotNull LazyEvaluator timer = new LazyEvaluator<>(() -> new Timer(true)); private final @NotNull AutoClosableReentrantLock timerLock = new AutoClosableReentrantLock(); private final @NotNull IScopes scopes; private final boolean enableSessionTracking; @@ -105,21 +106,19 @@ public void onBackground() { private void scheduleEndSession() { try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) { cancelTask(); - if (timer != null) { - timerTask = - new TimerTask() { - @Override - public void run() { - if (enableSessionTracking) { - scopes.endSession(); - } - scopes.getOptions().getReplayController().stop(); - scopes.getOptions().getContinuousProfiler().close(false); + timerTask = + new TimerTask() { + @Override + public void run() { + if (enableSessionTracking) { + scopes.endSession(); } - }; + scopes.getOptions().getReplayController().stop(); + scopes.getOptions().getContinuousProfiler().close(false); + } + }; - timer.schedule(timerTask, sessionIntervalMillis); - } + timer.getValue().schedule(timerTask, sessionIntervalMillis); } } @@ -152,6 +151,6 @@ TimerTask getTimerTask() { @TestOnly @NotNull Timer getTimer() { - return timer; + return timer.getValue(); } } diff --git a/sentry/src/main/java/io/sentry/NoOpScope.java b/sentry/src/main/java/io/sentry/NoOpScope.java index d996fa29d59..dd1a202b548 100644 --- a/sentry/src/main/java/io/sentry/NoOpScope.java +++ b/sentry/src/main/java/io/sentry/NoOpScope.java @@ -5,6 +5,7 @@ import io.sentry.protocol.Request; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; +import io.sentry.util.LazyEvaluator; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; @@ -20,7 +21,8 @@ public final class NoOpScope implements IScope { private static final NoOpScope instance = new NoOpScope(); - private final @NotNull SentryOptions emptyOptions = SentryOptions.empty(); + private final @NotNull LazyEvaluator emptyOptions = + new LazyEvaluator<>(() -> SentryOptions.empty()); private NoOpScope() {} @@ -229,7 +231,7 @@ public void withTransaction(Scope.@NotNull IWithTransaction callback) {} @ApiStatus.Internal @Override public @NotNull SentryOptions getOptions() { - return emptyOptions; + return emptyOptions.getValue(); } @ApiStatus.Internal diff --git a/sentry/src/main/java/io/sentry/NoOpScopes.java b/sentry/src/main/java/io/sentry/NoOpScopes.java index e1cc49a34a7..34adb15769b 100644 --- a/sentry/src/main/java/io/sentry/NoOpScopes.java +++ b/sentry/src/main/java/io/sentry/NoOpScopes.java @@ -7,6 +7,7 @@ import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.transport.RateLimiter; +import io.sentry.util.LazyEvaluator; import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -16,7 +17,8 @@ public final class NoOpScopes implements IScopes { private static final NoOpScopes instance = new NoOpScopes(); - private final @NotNull SentryOptions emptyOptions = SentryOptions.empty(); + private final @NotNull LazyEvaluator emptyOptions = + new LazyEvaluator<>(() -> SentryOptions.empty()); private NoOpScopes() {} @@ -274,7 +276,7 @@ public void setActiveSpan(final @Nullable ISpan span) {} @Override public @NotNull SentryOptions getOptions() { - return emptyOptions; + return emptyOptions.getValue(); } @Override From 297e1f3573b5b9c58b3a4d6e0ca8c6cfee4c6545 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 4 Aug 2025 13:43:46 +0200 Subject: [PATCH 32/43] Pre-allocate extras dictionary with the actual size --- .../sentry/android/core/SystemEventsBreadcrumbsIntegration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index a06a6adeb2e..eb212919166 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -398,8 +398,8 @@ String getStringAfterDotFast(final @Nullable String str) { } } else { final Bundle extras = intent.getExtras(); - final Map newExtras = new HashMap<>(); if (extras != null && !extras.isEmpty()) { + final Map newExtras = new HashMap<>(extras.size()); for (String item : extras.keySet()) { try { @SuppressWarnings("deprecation") From 9b55d0635de7a95c3524cf454371f3e21bfc3e22 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Mon, 4 Aug 2025 23:34:10 +0200 Subject: [PATCH 33/43] Prewarm SentryExecutorService and make it bounded --- .../api/sentry-test-support.api | 2 + .../src/main/kotlin/io/sentry/test/Mocks.kt | 4 + sentry/api/sentry.api | 3 + sentry/build.gradle.kts | 1 + .../io/sentry/ISentryExecutorService.java | 3 + .../io/sentry/NoOpSentryExecutorService.java | 3 + sentry/src/main/java/io/sentry/Sentry.java | 3 +- .../java/io/sentry/SentryExecutorService.java | 111 +++++++++++- .../main/java/io/sentry/SentryOptions.java | 3 +- .../java/io/sentry/SpotlightIntegration.java | 2 +- .../sentry/logger/LoggerBatchProcessor.java | 2 +- .../io/sentry/SentryExecutorServiceTest.kt | 164 +++++++++++++++--- 12 files changed, 269 insertions(+), 32 deletions(-) diff --git a/sentry-test-support/api/sentry-test-support.api b/sentry-test-support/api/sentry-test-support.api index 2623a7813d2..ce9d0506ea6 100644 --- a/sentry-test-support/api/sentry-test-support.api +++ b/sentry-test-support/api/sentry-test-support.api @@ -20,6 +20,7 @@ public final class io/sentry/test/DeferredExecutorService : io/sentry/ISentryExe public fun close (J)V public final fun hasScheduledRunnables ()Z public fun isClosed ()Z + public fun prewarm ()V public final fun runAll ()V public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; @@ -30,6 +31,7 @@ public final class io/sentry/test/ImmediateExecutorService : io/sentry/ISentryEx public fun ()V public fun close (J)V public fun isClosed ()Z + public fun prewarm ()V public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt index 49189f183e3..09d5d181ec4 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt @@ -35,6 +35,8 @@ class ImmediateExecutorService : ISentryExecutorService { override fun close(timeoutMillis: Long) {} override fun isClosed(): Boolean = false + + override fun prewarm() = Unit } class DeferredExecutorService : ISentryExecutorService { @@ -72,6 +74,8 @@ class DeferredExecutorService : ISentryExecutorService { override fun isClosed(): Boolean = false + override fun prewarm() = Unit + fun hasScheduledRunnables(): Boolean = scheduledRunnables.isNotEmpty() } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index b5dcbe2caf6..40ba123f0cf 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1030,6 +1030,7 @@ public abstract interface class io/sentry/ISentryClient { public abstract interface class io/sentry/ISentryExecutorService { public abstract fun close (J)V public abstract fun isClosed ()Z + public abstract fun prewarm ()V public abstract fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public abstract fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public abstract fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; @@ -2954,8 +2955,10 @@ public final class io/sentry/SentryExceptionFactory { public final class io/sentry/SentryExecutorService : io/sentry/ISentryExecutorService { public fun ()V + public fun (Lio/sentry/SentryOptions;)V public fun close (J)V public fun isClosed ()Z + public fun prewarm ()V public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; diff --git a/sentry/build.gradle.kts b/sentry/build.gradle.kts index 6a5a2182a89..bad0ea56e50 100644 --- a/sentry/build.gradle.kts +++ b/sentry/build.gradle.kts @@ -53,6 +53,7 @@ tasks { dependsOn(jacocoTestReport) } test { + jvmArgs("--add-opens", "java.base/java.util.concurrent=ALL-UNNAMED") environment["SENTRY_TEST_PROPERTY"] = "\"some-value\"" environment["SENTRY_TEST_MAP_KEY1"] = "\"value1\"" environment["SENTRY_TEST_MAP_KEY2"] = "value2" diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 9bdef8db2b7..ca3352d93a9 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -45,4 +45,7 @@ Future schedule(final @NotNull Runnable runnable, final long delayMillis) * @return If the executorService was previously closed */ boolean isClosed(); + + /** Pre-warms the executor service by increasing the initial queue capacity */ + void prewarm(); } diff --git a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java index 735c0dc29a3..d1dc6b207e5 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java @@ -36,4 +36,7 @@ public void close(long timeoutMillis) {} public boolean isClosed() { return false; } + + @Override + public void prewarm() {} } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 0596f8f2ffb..202a1e3b343 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -343,7 +343,8 @@ private static void init(final @NotNull SentryOptions options, final boolean glo // to // set a new one if (options.getExecutorService().isClosed()) { - options.setExecutorService(new SentryExecutorService()); + options.setExecutorService(new SentryExecutorService(options)); + options.getExecutorService().prewarm(); } // when integrations are registered on Scopes ctor and async integrations are fired, // it might and actually happened that integrations called captureSomething diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index bb08e51b3a0..a1fe94930e3 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -2,43 +2,96 @@ import io.sentry.util.AutoClosableReentrantLock; import java.util.concurrent.Callable; -import java.util.concurrent.Executors; +import java.util.concurrent.CancellationException; import java.util.concurrent.Future; -import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @ApiStatus.Internal public final class SentryExecutorService implements ISentryExecutorService { - private final @NotNull ScheduledExecutorService executorService; + /** + * ScheduledThreadPoolExecutor grows work queue by 50% each time. With the initial capacity of 16 + * it will have to resize 4 times to reach 40, which is a decent middle-ground for prewarming. + * This will prevent from growing in unexpected areas of the SDK. + */ + private static final int INITIAL_QUEUE_SIZE = 40; + + /** + * By default, the work queue is unbounded so it can grow as much as the memory allows. We want to + * limit it by 271 which would be x8 times growth from the default initial capacity. + */ + private static final int MAX_QUEUE_SIZE = 271; + + private final @NotNull ScheduledThreadPoolExecutor executorService; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); + @SuppressWarnings("UnnecessaryLambda") + private final @NotNull Runnable dummyRunnable = () -> {}; + + private final @Nullable SentryOptions options; + @TestOnly - SentryExecutorService(final @NotNull ScheduledExecutorService executorService) { + SentryExecutorService( + final @NotNull ScheduledThreadPoolExecutor executorService, + final @Nullable SentryOptions options) { this.executorService = executorService; + this.options = options; + } + + public SentryExecutorService(final @Nullable SentryOptions options) { + this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), options); } public SentryExecutorService() { - this(Executors.newSingleThreadScheduledExecutor(new SentryExecutorServiceThreadFactory())); + this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), null); } @Override public @NotNull Future submit(final @NotNull Runnable runnable) { - return executorService.submit(runnable); + if (executorService.getQueue().size() < MAX_QUEUE_SIZE) { + return executorService.submit(runnable); + } + // TODO: maybe RejectedExecutionException? + if (options != null) { + options + .getLogger() + .log(SentryLevel.WARNING, "Task " + runnable + " rejected from " + executorService); + } + return new CancelledFuture<>(); } @Override public @NotNull Future submit(final @NotNull Callable callable) { - return executorService.submit(callable); + if (executorService.getQueue().size() < MAX_QUEUE_SIZE) { + return executorService.submit(callable); + } + // TODO: maybe RejectedExecutionException? + if (options != null) { + options + .getLogger() + .log(SentryLevel.WARNING, "Task " + callable + " rejected from " + executorService); + } + return new CancelledFuture<>(); } @Override public @NotNull Future schedule(final @NotNull Runnable runnable, final long delayMillis) { - return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS); + if (executorService.getQueue().size() < MAX_QUEUE_SIZE) { + return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS); + } + // TODO: maybe RejectedExecutionException? + if (options != null) { + options + .getLogger() + .log(SentryLevel.WARNING, "Task " + runnable + " rejected from " + executorService); + } + return new CancelledFuture<>(); } @Override @@ -65,6 +118,21 @@ public boolean isClosed() { } } + @SuppressWarnings({"FutureReturnValueIgnored"}) + @Override + public void prewarm() { + executorService.submit( + () -> { + // schedule a bunch of dummy runnables in the future that will never execute to trigger + // queue + // growth and then clear the queue up + for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { + executorService.schedule(dummyRunnable, Long.MAX_VALUE, TimeUnit.DAYS); + } + executorService.getQueue().clear(); + }); + } + private static final class SentryExecutorServiceThreadFactory implements ThreadFactory { private int cnt; @@ -75,4 +143,31 @@ private static final class SentryExecutorServiceThreadFactory implements ThreadF return ret; } } + + private static final class CancelledFuture implements Future { + @Override + public boolean cancel(final boolean mayInterruptIfRunning) { + return true; + } + + @Override + public boolean isCancelled() { + return true; + } + + @Override + public boolean isDone() { + return true; + } + + @Override + public T get() { + throw new CancellationException(); + } + + @Override + public T get(final long timeout, final @NotNull TimeUnit unit) { + throw new CancellationException(); + } + } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index cb306f0aaf3..44b2c8cd36d 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -3040,7 +3040,8 @@ private SentryOptions(final boolean empty) { setSpanFactory(SpanFactoryFactory.create(new LoadClass(), NoOpLogger.getInstance())); // SentryExecutorService should be initialized before any // SendCachedEventFireAndForgetIntegration - executorService = new SentryExecutorService(); + executorService = new SentryExecutorService(this); + executorService.prewarm(); // UncaughtExceptionHandlerIntegration should be inited before any other Integration. // if there's an error on the setup, we are able to capture it diff --git a/sentry/src/main/java/io/sentry/SpotlightIntegration.java b/sentry/src/main/java/io/sentry/SpotlightIntegration.java index 910259ad131..1eb9ed83ae0 100644 --- a/sentry/src/main/java/io/sentry/SpotlightIntegration.java +++ b/sentry/src/main/java/io/sentry/SpotlightIntegration.java @@ -32,7 +32,7 @@ public void register(@NotNull IScopes scopes, @NotNull SentryOptions options) { this.logger = options.getLogger(); if (options.getBeforeEnvelopeCallback() == null && options.isEnableSpotlight()) { - executorService = new SentryExecutorService(); + executorService = new SentryExecutorService(options); options.setBeforeEnvelopeCallback(this); logger.log(DEBUG, "SpotlightIntegration enabled."); addIntegrationToSdkVersion("Spotlight"); diff --git a/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java b/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java index 5aa03a74118..3d760263f38 100644 --- a/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java +++ b/sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java @@ -40,7 +40,7 @@ public LoggerBatchProcessor( this.options = options; this.client = client; this.queue = new ConcurrentLinkedQueue<>(); - this.executorService = new SentryExecutorService(); + this.executorService = new SentryExecutorService(options); } @Override diff --git a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt index 18cc73986a4..19dd60469cc 100644 --- a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt +++ b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt @@ -1,13 +1,20 @@ package io.sentry -import java.util.concurrent.Executors -import java.util.concurrent.ScheduledExecutorService +import io.sentry.test.getProperty +import java.util.concurrent.BlockingQueue +import java.util.concurrent.Callable +import java.util.concurrent.CancellationException +import java.util.concurrent.LinkedBlockingQueue +import java.util.concurrent.ScheduledThreadPoolExecutor import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue import org.awaitility.kotlin.await import org.mockito.kotlin.any +import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -16,24 +23,24 @@ import org.mockito.kotlin.whenever class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards submit call to ExecutorService`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock { on { queue } doReturn LinkedBlockingQueue() } + val sentryExecutor = SentryExecutorService(executor, null) sentryExecutor.submit {} verify(executor).submit(any()) } @Test fun `SentryExecutorService forwards schedule call to ExecutorService`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock { on { queue } doReturn LinkedBlockingQueue() } + val sentryExecutor = SentryExecutorService(executor, null) sentryExecutor.schedule({}, 0L) verify(executor).schedule(any(), any(), any()) } @Test fun `SentryExecutorService forwards close call to ExecutorService`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock() + val sentryExecutor = SentryExecutorService(executor, null) whenever(executor.isShutdown).thenReturn(false) whenever(executor.awaitTermination(any(), any())).thenReturn(true) sentryExecutor.close(15000) @@ -42,8 +49,8 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close and call shutdownNow if not enough time`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock() + val sentryExecutor = SentryExecutorService(executor, null) whenever(executor.isShutdown).thenReturn(false) whenever(executor.awaitTermination(any(), any())).thenReturn(false) sentryExecutor.close(15000) @@ -52,8 +59,8 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close and call shutdownNow if await throws`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock() + val sentryExecutor = SentryExecutorService(executor, null) whenever(executor.isShutdown).thenReturn(false) whenever(executor.awaitTermination(any(), any())).thenThrow(InterruptedException()) sentryExecutor.close(15000) @@ -62,8 +69,8 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close but do not shutdown if its already closed`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock() + val sentryExecutor = SentryExecutorService(executor, null) whenever(executor.isShutdown).thenReturn(true) sentryExecutor.close(15000) verify(executor, never()).shutdown() @@ -71,8 +78,8 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService forwards close call to ExecutorService and close it`() { - val executor = Executors.newSingleThreadScheduledExecutor() - val sentryExecutor = SentryExecutorService(executor) + val executor = ScheduledThreadPoolExecutor(1) + val sentryExecutor = SentryExecutorService(executor, null) sentryExecutor.close(15000) assertTrue(executor.isShutdown) } @@ -88,17 +95,134 @@ class SentryExecutorServiceTest { @Test fun `SentryExecutorService isClosed returns true if executor is shutdown`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock() + val sentryExecutor = SentryExecutorService(executor, null) whenever(executor.isShutdown).thenReturn(true) assertTrue(sentryExecutor.isClosed) } @Test fun `SentryExecutorService isClosed returns false if executor is not shutdown`() { - val executor = mock() - val sentryExecutor = SentryExecutorService(executor) + val executor = mock() + val sentryExecutor = SentryExecutorService(executor, null) whenever(executor.isShutdown).thenReturn(false) assertFalse(sentryExecutor.isClosed) } + + @Test + fun `SentryExecutorService submit runnable returns cancelled future when queue size exceeds limit`() { + val queue = mock>() + whenever(queue.size).thenReturn(272) // Above MAX_QUEUE_SIZE (271) + + val executor = mock { on { getQueue() } doReturn queue } + + val options = mock() + val logger = mock() + whenever(options.logger).thenReturn(logger) + + val sentryExecutor = SentryExecutorService(executor, options) + val future = sentryExecutor.submit {} + + assertTrue(future.isCancelled) + assertTrue(future.isDone) + assertFailsWith { future.get() } + verify(executor, never()).submit(any()) + verify(logger).log(any(), any()) + } + + @Test + fun `SentryExecutorService submit runnable accepts when queue size is within limit`() { + val queue = mock>() + whenever(queue.size).thenReturn(270) // Below MAX_QUEUE_SIZE (271) + + val executor = mock { on { getQueue() } doReturn queue } + + val sentryExecutor = SentryExecutorService(executor, null) + sentryExecutor.submit {} + + verify(executor).submit(any()) + } + + @Test + fun `SentryExecutorService submit callable returns cancelled future when queue size exceeds limit`() { + val queue = mock>() + whenever(queue.size).thenReturn(272) // Above MAX_QUEUE_SIZE (271) + + val executor = mock { on { getQueue() } doReturn queue } + + val options = mock() + val logger = mock() + whenever(options.logger).thenReturn(logger) + + val sentryExecutor = SentryExecutorService(executor, options) + val future = sentryExecutor.submit(Callable { "result" }) + + assertTrue(future.isCancelled) + assertTrue(future.isDone) + assertFailsWith { future.get() } + verify(executor, never()).submit(any>()) + verify(logger).log(any(), any()) + } + + @Test + fun `SentryExecutorService submit callable accepts when queue size is within limit`() { + val queue = mock>() + whenever(queue.size).thenReturn(270) // Below MAX_QUEUE_SIZE (271) + + val executor = mock { on { getQueue() } doReturn queue } + + val sentryExecutor = SentryExecutorService(executor, null) + sentryExecutor.submit(Callable { "result" }) + + verify(executor).submit(any>()) + } + + @Test + fun `SentryExecutorService schedule returns cancelled future when queue size exceeds limit`() { + val queue = mock>() + whenever(queue.size).thenReturn(272) // Above MAX_QUEUE_SIZE (271) + + val executor = mock { on { getQueue() } doReturn queue } + + val options = mock() + val logger = mock() + whenever(options.logger).thenReturn(logger) + + val sentryExecutor = SentryExecutorService(executor, options) + val future = sentryExecutor.schedule({}, 1000L) + + assertTrue(future.isCancelled) + assertTrue(future.isDone) + assertFailsWith { future.get() } + verify(executor, never()).schedule(any(), any(), any()) + verify(logger).log(any(), any()) + } + + @Test + fun `SentryExecutorService schedule accepts when queue size is within limit`() { + val queue = mock>() + whenever(queue.size).thenReturn(270) // Below MAX_QUEUE_SIZE (271) + + val executor = mock { on { getQueue() } doReturn queue } + + val sentryExecutor = SentryExecutorService(executor, null) + sentryExecutor.schedule({}, 1000L) + + verify(executor).schedule(any(), any(), any()) + } + + @Test + fun `SentryExecutorService prewarm schedules dummy tasks and clears queue`() { + val executor = ScheduledThreadPoolExecutor(1) + + val sentryExecutor = SentryExecutorService(executor, null) + sentryExecutor.prewarm() + + Thread.sleep(1000) + + // the internal queue/array should be resized 4 times to 54 + assertEquals(54, (executor.queue.getProperty("queue") as Array<*>).size) + // the queue should be empty + assertEquals(0, executor.queue.size) + } } From d20ab343ffd97e4ff93d07326a9d5913fc3e5cfe Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 5 Aug 2025 11:10:38 +0200 Subject: [PATCH 34/43] Use HandlerThread for receiving broadcasts --- .../io/sentry/android/core/ContextUtils.java | 15 ++++++---- .../sentry/android/core/DeviceInfoUtil.java | 2 +- .../SystemEventsBreadcrumbsIntegration.java | 29 +++++++++---------- .../android/core/AndroidProfilerTest.kt | 2 ++ .../core/AndroidTransactionProfilerTest.kt | 2 ++ .../sentry/android/core/ContextUtilsTest.kt | 4 +-- .../SystemEventsBreadcrumbsIntegrationTest.kt | 12 ++++---- 7 files changed, 38 insertions(+), 28 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java index 8ba0e16b7c6..60ae00f2ead 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ContextUtils.java @@ -15,6 +15,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.os.Build; +import android.os.Handler; import android.util.DisplayMetrics; import io.sentry.ILogger; import io.sentry.SentryLevel; @@ -455,8 +456,10 @@ public static boolean appIsLibraryForComposePreview(final @NotNull Context conte final @NotNull Context context, final @NotNull SentryOptions options, final @Nullable BroadcastReceiver receiver, - final @NotNull IntentFilter filter) { - return registerReceiver(context, new BuildInfoProvider(options.getLogger()), receiver, filter); + final @NotNull IntentFilter filter, + final @Nullable Handler handler) { + return registerReceiver( + context, new BuildInfoProvider(options.getLogger()), receiver, filter, handler); } /** Register an exported BroadcastReceiver, independently from platform version. */ @@ -465,15 +468,17 @@ public static boolean appIsLibraryForComposePreview(final @NotNull Context conte final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider, final @Nullable BroadcastReceiver receiver, - final @NotNull IntentFilter filter) { + final @NotNull IntentFilter filter, + final @Nullable Handler handler) { if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.TIRAMISU) { // From https://developer.android.com/guide/components/broadcasts#context-registered-receivers // If this receiver is listening for broadcasts sent from the system or from other apps, even // other apps that you own—use the RECEIVER_EXPORTED flag. If instead this receiver is // listening only for broadcasts sent by your app, use the RECEIVER_NOT_EXPORTED flag. - return context.registerReceiver(receiver, filter, Context.RECEIVER_NOT_EXPORTED); + return context.registerReceiver( + receiver, filter, null, handler, Context.RECEIVER_NOT_EXPORTED); } else { - return context.registerReceiver(receiver, filter); + return context.registerReceiver(receiver, filter, null, handler); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java b/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java index 5baf6e2a8d2..7ba321426a1 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java @@ -275,7 +275,7 @@ private Date getBootTime() { @Nullable private Intent getBatteryIntent() { return ContextUtils.registerReceiver( - context, buildInfoProvider, null, new IntentFilter(Intent.ACTION_BATTERY_CHANGED)); + context, buildInfoProvider, null, new IntentFilter(Intent.ACTION_BATTERY_CHANGED), null); } /** diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index eb212919166..094f15a6b5b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -25,6 +25,9 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.Process; import io.sentry.Breadcrumb; import io.sentry.Hint; import io.sentry.IScopes; @@ -139,7 +142,13 @@ private void registerReceiver( try { // registerReceiver can throw SecurityException but it's not documented in the // official docs - ContextUtils.registerReceiver(context, options, receiver, filter); + final @NotNull HandlerThread handlerThread = + new HandlerThread( + "SystemEventsReceiver", Process.THREAD_PRIORITY_BACKGROUND); + handlerThread.start(); + // onReceive will be called on this handler thread + final @NotNull Handler handler = new Handler(handlerThread.getLooper()); + ContextUtils.registerReceiver(context, options, receiver, filter, handler); if (!isReceiverRegistered.getAndSet(true)) { options .getLogger() @@ -326,25 +335,15 @@ public void onReceive(final Context context, final @NotNull Intent intent) { final BatteryState state = batteryState; final long now = System.currentTimeMillis(); - try { - options - .getExecutorService() - .submit( - () -> { - final Breadcrumb breadcrumb = createBreadcrumb(now, intent, action, state); - final Hint hint = new Hint(); - hint.set(ANDROID_INTENT, intent); - scopes.addBreadcrumb(breadcrumb, hint); - }); - } catch (Throwable t) { - // ignored - } + final Breadcrumb breadcrumb = createBreadcrumb(now, intent, action, state); + final Hint hint = new Hint(); + hint.set(ANDROID_INTENT, intent); + scopes.addBreadcrumb(breadcrumb, hint); } // in theory this should be ThreadLocal, but we won't have more than 1 thread accessing it, // so we save some memory here and CPU cycles. 64 is because all intent actions we subscribe for // are less than 64 chars. We also don't care about encoding as those are always UTF. - // TODO: _MULTI_THREADED_EXECUTOR_ private final char[] buf = new char[64]; @TestOnly diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt index 998a24b3d2c..f4b4da814b8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidProfilerTest.kt @@ -80,6 +80,8 @@ class AndroidProfilerTest { override fun close(timeoutMillis: Long) {} override fun isClosed() = false + + override fun prewarm() = Unit } val options = diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 0490bc30d09..ec6ba18d65e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -89,6 +89,8 @@ class AndroidTransactionProfilerTest { override fun close(timeoutMillis: Long) {} override fun isClosed() = false + + override fun prewarm() = Unit } val options = diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt index b9bb6170e44..57186bdf64c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt @@ -221,7 +221,7 @@ class ContextUtilsTest { val filter = mock() val context = mock() whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.S) - ContextUtils.registerReceiver(context, buildInfo, receiver, filter) + ContextUtils.registerReceiver(context, buildInfo, receiver, filter, null) verify(context).registerReceiver(eq(receiver), eq(filter)) } @@ -232,7 +232,7 @@ class ContextUtilsTest { val filter = mock() val context = mock() whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.TIRAMISU) - ContextUtils.registerReceiver(context, buildInfo, receiver, filter) + ContextUtils.registerReceiver(context, buildInfo, receiver, filter, null) verify(context).registerReceiver(eq(receiver), eq(filter), eq(Context.RECEIVER_NOT_EXPORTED)) } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index c23130322ae..cc1519d146c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -89,7 +89,7 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.context).registerReceiver(any(), any(), any()) + verify(fixture.context).registerReceiver(any(), any(), anyOrNull(), anyOrNull(), any()) assertNotNull(sut.receiver) } @@ -299,7 +299,8 @@ class SystemEventsBreadcrumbsIntegrationTest { @Test fun `Do not crash if registerReceiver throws exception`() { val sut = fixture.getSut() - whenever(fixture.context.registerReceiver(any(), any(), any())).thenThrow(SecurityException()) + whenever(fixture.context.registerReceiver(any(), any(), anyOrNull(), anyOrNull(), any())) + .thenThrow(SecurityException()) sut.register(fixture.scopes, fixture.options) @@ -448,12 +449,13 @@ class SystemEventsBreadcrumbsIntegrationTest { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) - verify(fixture.context).registerReceiver(any(), any(), any()) + verify(fixture.context).registerReceiver(any(), any(), anyOrNull(), anyOrNull(), any()) sut.onBackground() sut.onForeground() - verify(fixture.context, times(2)).registerReceiver(any(), any(), any()) + verify(fixture.context, times(2)) + .registerReceiver(any(), any(), anyOrNull(), anyOrNull(), any()) assertNotNull(sut.receiver) } @@ -462,7 +464,7 @@ class SystemEventsBreadcrumbsIntegrationTest { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) - verify(fixture.context).registerReceiver(any(), any(), any()) + verify(fixture.context).registerReceiver(any(), any(), anyOrNull(), anyOrNull(), any()) val receiver = sut.receiver sut.onForeground() From a5702e0230349b82dae428a902b63c87e7071ffd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 5 Aug 2025 11:18:55 +0200 Subject: [PATCH 35/43] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a90ced0c451..de28d57580d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Internal - Use single `LifecycleObserver` and multi-cast it to the integrations interested in lifecycle states ([#4567](https://github.com/getsentry/sentry-java/pull/4567)) +- Prewarm `SentryExecutorService` for better performance at runtime ([#4606](https://github.com/getsentry/sentry-java/pull/4606)) ### Fixes From a0e161a425d33757d7dc93e9da01787d3ae70a78 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 5 Aug 2025 09:37:42 +0000 Subject: [PATCH 36/43] Update registerReceiver method calls to match Android SDK changes Co-authored-by: roman.zavarnitsyn --- .../src/test/java/io/sentry/android/core/ContextUtilsTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt index 57186bdf64c..a190ea9aad8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt @@ -222,7 +222,7 @@ class ContextUtilsTest { val context = mock() whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.S) ContextUtils.registerReceiver(context, buildInfo, receiver, filter, null) - verify(context).registerReceiver(eq(receiver), eq(filter)) + verify(context).registerReceiver(eq(receiver), eq(filter), isNull(), isNull()) } @Test @@ -233,7 +233,7 @@ class ContextUtilsTest { val context = mock() whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.TIRAMISU) ContextUtils.registerReceiver(context, buildInfo, receiver, filter, null) - verify(context).registerReceiver(eq(receiver), eq(filter), eq(Context.RECEIVER_NOT_EXPORTED)) + verify(context).registerReceiver(eq(receiver), eq(filter), isNull(), isNull(), eq(Context.RECEIVER_NOT_EXPORTED)) } @Test From 74bc8a65f0d0cfe97fd9f1df6c0d67d91b91c08b Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 5 Aug 2025 09:40:41 +0000 Subject: [PATCH 37/43] Format code --- .../test/java/io/sentry/android/core/ContextUtilsTest.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt index a190ea9aad8..e5de1336894 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ContextUtilsTest.kt @@ -233,7 +233,14 @@ class ContextUtilsTest { val context = mock() whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.TIRAMISU) ContextUtils.registerReceiver(context, buildInfo, receiver, filter, null) - verify(context).registerReceiver(eq(receiver), eq(filter), isNull(), isNull(), eq(Context.RECEIVER_NOT_EXPORTED)) + verify(context) + .registerReceiver( + eq(receiver), + eq(filter), + isNull(), + isNull(), + eq(Context.RECEIVER_NOT_EXPORTED), + ) } @Test From c2447dfb74f49d843e431dd3bd00d7463fe011f2 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 6 Aug 2025 14:47:57 +0200 Subject: [PATCH 38/43] Add a comment --- sentry/src/main/java/io/sentry/ISentryExecutorService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index ca3352d93a9..4e49ab56550 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -46,6 +46,8 @@ Future schedule(final @NotNull Runnable runnable, final long delayMillis) */ boolean isClosed(); - /** Pre-warms the executor service by increasing the initial queue capacity */ + /** Pre-warms the executor service by increasing the initial queue capacity. SHOULD be called + * directly after instantiating this executor service. + */ void prewarm(); } From 36482b99081fb16e938e9455960d59e70da7f2b0 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 6 Aug 2025 16:23:27 +0200 Subject: [PATCH 39/43] Do not leak HandlerThread in SystemEventsReceiver --- .../core/SystemEventsBreadcrumbsIntegration.java | 14 ++++++++++---- .../java/io/sentry/ISentryExecutorService.java | 3 ++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index b5ea809040f..54d53c8fc8d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -66,6 +66,7 @@ public final class SystemEventsBreadcrumbsIntegration private volatile boolean isClosed = false; private volatile boolean isStopped = false; private volatile IntentFilter filter = null; + private volatile HandlerThread handlerThread = null; private final @NotNull AtomicBoolean isReceiverRegistered = new AtomicBoolean(false); private final @NotNull AutoClosableReentrantLock receiverLock = new AutoClosableReentrantLock(); // Track previous battery state to avoid duplicate breadcrumbs when values haven't changed @@ -141,13 +142,16 @@ private void registerReceiver( filter.addAction(item); } } - try { - // registerReceiver can throw SecurityException but it's not documented in the - // official docs - final @NotNull HandlerThread handlerThread = + if (handlerThread == null) { + handlerThread = new HandlerThread( "SystemEventsReceiver", Process.THREAD_PRIORITY_BACKGROUND); handlerThread.start(); + } + try { + // registerReceiver can throw SecurityException but it's not documented in the + // official docs + // onReceive will be called on this handler thread final @NotNull Handler handler = new Handler(handlerThread.getLooper()); ContextUtils.registerReceiver(context, options, receiver, filter, handler); @@ -204,6 +208,8 @@ public void close() throws IOException { try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { isClosed = true; filter = null; + handlerThread.quit(); + handlerThread = null; } AppState.getInstance().removeAppStateListener(this); diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 4e49ab56550..ffad05361f6 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -46,7 +46,8 @@ Future schedule(final @NotNull Runnable runnable, final long delayMillis) */ boolean isClosed(); - /** Pre-warms the executor service by increasing the initial queue capacity. SHOULD be called + /** + * Pre-warms the executor service by increasing the initial queue capacity. SHOULD be called * directly after instantiating this executor service. */ void prewarm(); From 194ee544e4231f476cb5fdb976d4e75138e15dcc Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 6 Aug 2025 16:44:29 +0200 Subject: [PATCH 40/43] Null-check handlerThread --- .../android/core/SystemEventsBreadcrumbsIntegration.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 54d53c8fc8d..91099d01253 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -208,7 +208,9 @@ public void close() throws IOException { try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { isClosed = true; filter = null; - handlerThread.quit(); + if (handlerThread != null) { + handlerThread.quit(); + } handlerThread = null; } From 9021c7832e889bdeefe755baa9d3cb4c34902657 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 6 Aug 2025 16:49:27 +0200 Subject: [PATCH 41/43] Clear the queue in a different task --- sentry/src/main/java/io/sentry/SentryExecutorService.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index a1fe94930e3..95ac37bf140 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -129,8 +129,10 @@ public void prewarm() { for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { executorService.schedule(dummyRunnable, Long.MAX_VALUE, TimeUnit.DAYS); } - executorService.getQueue().clear(); }); + executorService.submit(() -> { + executorService.getQueue().clear(); + }); } private static final class SentryExecutorServiceThreadFactory implements ThreadFactory { From 0e6a1f4f875192ea3a57c3c050fff51bc337e587 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 6 Aug 2025 16:56:29 +0200 Subject: [PATCH 42/43] Formatting --- sentry/src/main/java/io/sentry/SentryExecutorService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index 95ac37bf140..c7700ee36bd 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -130,9 +130,10 @@ public void prewarm() { executorService.schedule(dummyRunnable, Long.MAX_VALUE, TimeUnit.DAYS); } }); - executorService.submit(() -> { - executorService.getQueue().clear(); - }); + executorService.submit( + () -> { + executorService.getQueue().clear(); + }); } private static final class SentryExecutorServiceThreadFactory implements ThreadFactory { From face07184c4231e00375900b17cbbb9daf616721 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 7 Aug 2025 11:34:17 +0200 Subject: [PATCH 43/43] Cancel tasks and purge them --- .../java/io/sentry/SentryExecutorService.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index c7700ee36bd..3fe262b5538 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -4,6 +4,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -123,17 +124,18 @@ public boolean isClosed() { public void prewarm() { executorService.submit( () -> { - // schedule a bunch of dummy runnables in the future that will never execute to trigger - // queue - // growth and then clear the queue up - for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { - executorService.schedule(dummyRunnable, Long.MAX_VALUE, TimeUnit.DAYS); + try { + // schedule a bunch of dummy runnables in the future that will never execute to trigger + // queue growth and then purge the queue + for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { + final Future future = executorService.schedule(dummyRunnable, 365L, TimeUnit.DAYS); + future.cancel(true); + } + executorService.purge(); + } catch (RejectedExecutionException ignored) { + // ignore } }); - executorService.submit( - () -> { - executorService.getQueue().clear(); - }); } private static final class SentryExecutorServiceThreadFactory implements ThreadFactory {