-
-
Notifications
You must be signed in to change notification settings - Fork 458
[ANR] Update Connection Status cache in the background #4832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
53a1054
e520ff7
7ae75a2
e44381d
8dba1d8
5e5a8d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ public final class AndroidConnectionStatusProvider | |
| private static final @NotNull AutoClosableReentrantLock childCallbacksLock = | ||
| new AutoClosableReentrantLock(); | ||
| private static final @NotNull List<NetworkCallback> childCallbacks = new ArrayList<>(); | ||
| private static final AtomicBoolean isUpdatingCache = new AtomicBoolean(false); | ||
|
|
||
| private static final int[] transports = { | ||
| NetworkCapabilities.TRANSPORT_WIFI, | ||
|
|
@@ -268,10 +269,19 @@ private void updateCacheAndNotifyObservers( | |
|
|
||
| // Only notify observers if something meaningful changed | ||
| if (shouldUpdate) { | ||
| updateCache(networkCapabilities); | ||
|
|
||
| final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| cachedNetworkCapabilities = networkCapabilities; | ||
| lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); | ||
| final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.DEBUG, | ||
| "Cache updated - Status: " | ||
| + status | ||
| + ", Type: " | ||
| + getConnectionTypeFromCache()); | ||
|
|
||
| for (final @NotNull IConnectionStatusObserver observer : | ||
| connectionStatusObservers) { | ||
| observer.onConnectionStatusChanged(status); | ||
|
|
@@ -349,56 +359,57 @@ private boolean hasSignificantTransportChanges( | |
| } | ||
|
|
||
| @SuppressLint({"NewApi", "MissingPermission"}) | ||
| private void updateCache(@Nullable NetworkCapabilities networkCapabilities) { | ||
| private void updateCache() { | ||
| 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; | ||
| } | ||
| cachedNetworkCapabilities = null; | ||
| lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); | ||
| } | ||
| try { | ||
| if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) { | ||
| options | ||
| .getLogger() | ||
| .log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status."); | ||
| return; | ||
| } | ||
|
|
||
| if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) { | ||
| cachedNetworkCapabilities = null; | ||
| lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); | ||
| return; | ||
| } | ||
| if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) { | ||
| 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 { | ||
| cachedNetworkCapabilities = | ||
| null; // Clear cached capabilities if connectivity manager is null | ||
| } | ||
| } | ||
| lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); | ||
| // Fallback: query current active network in the background | ||
| submitSafe( | ||
| () -> { | ||
| // Avoid concurrent updates | ||
| if (!isUpdatingCache.getAndSet(true)) { | ||
| final ConnectivityManager connectivityManager = | ||
| getConnectivityManager(context, options.getLogger()); | ||
| if (connectivityManager != null) { | ||
| final @Nullable NetworkCapabilities capabilities = | ||
| getNetworkCapabilities(connectivityManager); | ||
|
|
||
| try (final @NotNull ISentryLifecycleToken ignored2 = lock.acquire()) { | ||
| cachedNetworkCapabilities = capabilities; | ||
| lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); | ||
|
|
||
| if (capabilities != null) { | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.DEBUG, | ||
| "Cache updated - Status: " | ||
| + getConnectionStatusFromCache() | ||
| + ", Type: " | ||
| + getConnectionTypeFromCache()); | ||
| } | ||
| } | ||
| } | ||
| isUpdatingCache.set(false); | ||
| } | ||
| }); | ||
|
|
||
| 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); | ||
| } catch (Throwable t) { | ||
| options.getLogger().log(SentryLevel.WARNING, "Failed to update connection status cache", t); | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Cache Update Timing Causes Stale Data
Additional Locations (1) |
||
| cachedNetworkCapabilities = null; | ||
| lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); | ||
| } | ||
|
|
@@ -412,15 +423,15 @@ private boolean isCacheValid() { | |
| @Override | ||
| public @NotNull ConnectionStatus getConnectionStatus() { | ||
| if (!isCacheValid()) { | ||
| updateCache(null); | ||
| updateCache(); | ||
| } | ||
| return getConnectionStatusFromCache(); | ||
| } | ||
|
|
||
| @Override | ||
| public @Nullable String getConnectionType() { | ||
| if (!isCacheValid()) { | ||
| updateCache(null); | ||
| updateCache(); | ||
| } | ||
| return getConnectionTypeFromCache(); | ||
| } | ||
|
|
@@ -490,7 +501,7 @@ public void onForeground() { | |
| () -> { | ||
| // proactively update cache and notify observers on foreground to ensure connectivity | ||
| // state is not stale | ||
| updateCache(null); | ||
| updateCache(); | ||
|
|
||
| final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); | ||
| if (status == ConnectionStatus.DISCONNECTED) { | ||
|
|
@@ -575,6 +586,14 @@ public NetworkCapabilities getCachedNetworkCapabilities() { | |
| } | ||
| } | ||
|
|
||
| @RequiresApi(Build.VERSION_CODES.M) | ||
| @SuppressLint("MissingPermission") | ||
| private static @Nullable NetworkCapabilities getNetworkCapabilities( | ||
| final @NotNull ConnectivityManager connectivityManager) { | ||
| final Network activeNetwork = connectivityManager.getActiveNetwork(); | ||
| return activeNetwork != null ? connectivityManager.getNetworkCapabilities(activeNetwork) : null; | ||
| } | ||
|
|
||
| /** | ||
| * Check the connection type of the active network | ||
| * | ||
|
|
@@ -603,14 +622,7 @@ public NetworkCapabilities getCachedNetworkCapabilities() { | |
| boolean cellular = false; | ||
|
|
||
| if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) { | ||
|
|
||
| final Network activeNetwork = connectivityManager.getActiveNetwork(); | ||
| if (activeNetwork == null) { | ||
| logger.log(SentryLevel.INFO, "Network is null and cannot check network status"); | ||
| return null; | ||
| } | ||
| final NetworkCapabilities networkCapabilities = | ||
| connectivityManager.getNetworkCapabilities(activeNetwork); | ||
| final NetworkCapabilities networkCapabilities = getNetworkCapabilities(connectivityManager); | ||
| if (networkCapabilities == null) { | ||
| logger.log(SentryLevel.INFO, "NetworkCapabilities is null and cannot check network type"); | ||
| return null; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.