Skip to content

Commit 9a7ced4

Browse files
runningcodeclaude
andcommitted
fix(android): Treat an unpopulated connection cache as stale (JAVA-717)
lastCacheUpdateTime used 0 for "never populated" while being compared against SystemClock.uptimeMillis(), which starts at 0 at boot. For the first two minutes of every boot the empty cache therefore read as fresh, so getConnectionStatus() skipped updateCache() and fell through to the legacy activeNetworkInfo path instead of reading NetworkCapabilities. The window reopens after every unregisterNetworkCallback(), which reset the field to 0. Switching clocks does not fix this on its own: elapsedRealtimeNanos() also starts at 0 at boot. Any 0-means-unset long compared against a boot-relative clock has the same flaw; only epoch millis made it safe, because there 0 is 1970. The cache now holds a Deadline, so "never populated" is expired by construction and has no numeric value to get wrong. The provider takes an ElapsedRealtimeClock in place of ICurrentDateProvider, which is what a two-minute TTL wants: it must keep counting while the device sleeps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5d5db76 commit 9a7ced4

3 files changed

Lines changed: 49 additions & 32 deletions

File tree

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import io.sentry.android.core.internal.gestures.AndroidViewGestureTargetLocator;
3636
import io.sentry.android.core.internal.modules.AssetsModulesLoader;
3737
import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider;
38-
import io.sentry.android.core.internal.util.AndroidCurrentDateProvider;
3938
import io.sentry.android.core.internal.util.AndroidThreadChecker;
4039
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
4140
import io.sentry.android.core.performance.AppStartMetrics;
@@ -178,7 +177,7 @@ static void initializeIntegrationsAndProcessors(
178177
if (options.getConnectionStatusProvider() instanceof NoOpConnectionStatusProvider) {
179178
options.setConnectionStatusProvider(
180179
new AndroidConnectionStatusProvider(
181-
context, options, buildInfoProvider, AndroidCurrentDateProvider.getInstance()));
180+
context, options, buildInfoProvider, options.getElapsedRealtimeClock()));
182181
}
183182

184183
if (options.getCacheDirPath() != null) {

‎sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
import io.sentry.android.core.AppState;
2020
import io.sentry.android.core.BuildInfoProvider;
2121
import io.sentry.android.core.ContextUtils;
22-
import io.sentry.transport.ICurrentDateProvider;
22+
import io.sentry.time.Deadline;
23+
import io.sentry.time.ElapsedRealtimeClock;
2324
import io.sentry.util.AutoClosableReentrantLock;
2425
import java.util.ArrayList;
2526
import java.util.List;
27+
import java.util.concurrent.TimeUnit;
2628
import java.util.concurrent.atomic.AtomicBoolean;
2729
import org.jetbrains.annotations.ApiStatus;
2830
import org.jetbrains.annotations.NotNull;
@@ -41,7 +43,7 @@ public final class AndroidConnectionStatusProvider
4143
private final @NotNull Context context;
4244
private final @NotNull SentryOptions options;
4345
private final @NotNull BuildInfoProvider buildInfoProvider;
44-
private final @NotNull ICurrentDateProvider timeProvider;
46+
private final @NotNull ElapsedRealtimeClock clock;
4547
private final @NotNull List<IConnectionStatusObserver> connectionStatusObservers;
4648
private final @Nullable Handler handler;
4749
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
@@ -66,29 +68,30 @@ public final class AndroidConnectionStatusProvider
6668

6769
private volatile @Nullable NetworkCapabilities cachedNetworkCapabilities;
6870
private volatile @Nullable Network currentNetwork;
69-
private volatile long lastCacheUpdateTime = 0;
70-
private static final long CACHE_TTL_MS = 2 * 60 * 1000L; // 2 minutes
71+
private volatile @NotNull Deadline cacheFreshUntil;
72+
private static final long CACHE_TTL_MINUTES = 2;
7173
private final @NotNull AtomicBoolean isConnected = new AtomicBoolean(false);
7274

7375
public AndroidConnectionStatusProvider(
7476
@NotNull Context context,
7577
@NotNull SentryOptions options,
7678
@NotNull BuildInfoProvider buildInfoProvider,
77-
@NotNull ICurrentDateProvider timeProvider) {
78-
this(context, options, buildInfoProvider, timeProvider, null);
79+
@NotNull ElapsedRealtimeClock clock) {
80+
this(context, options, buildInfoProvider, clock, null);
7981
}
8082

8183
@SuppressLint("InlinedApi")
8284
public AndroidConnectionStatusProvider(
8385
@NotNull Context context,
8486
@NotNull SentryOptions options,
8587
@NotNull BuildInfoProvider buildInfoProvider,
86-
@NotNull ICurrentDateProvider timeProvider,
88+
@NotNull ElapsedRealtimeClock clock,
8789
@Nullable Handler handler) {
8890
this.context = ContextUtils.getApplicationContext(context);
8991
this.options = options;
9092
this.buildInfoProvider = buildInfoProvider;
91-
this.timeProvider = timeProvider;
93+
this.clock = clock;
94+
this.cacheFreshUntil = Deadline.passed(clock);
9295
this.handler = handler;
9396
this.connectionStatusObservers = new ArrayList<>();
9497

@@ -231,7 +234,7 @@ private void clearCacheAndNotifyObservers() {
231234
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
232235
cachedNetworkCapabilities = null;
233236
currentNetwork = null;
234-
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
237+
cacheFreshUntil = Deadline.after(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
235238

236239
options
237240
.getLogger()
@@ -362,13 +365,13 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) {
362365
SentryLevel.INFO,
363366
"No permission (ACCESS_NETWORK_STATE) to check network status.");
364367
cachedNetworkCapabilities = null;
365-
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
368+
cacheFreshUntil = Deadline.after(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
366369
return;
367370
}
368371

369372
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) {
370373
cachedNetworkCapabilities = null;
371-
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
374+
cacheFreshUntil = Deadline.after(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
372375
return;
373376
}
374377

@@ -387,7 +390,7 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) {
387390
null; // Clear cached capabilities if connectivity manager is null
388391
}
389392
}
390-
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
393+
cacheFreshUntil = Deadline.after(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
391394

392395
options
393396
.getLogger()
@@ -400,13 +403,13 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) {
400403
} catch (Throwable t) {
401404
options.getLogger().log(SentryLevel.WARNING, "Failed to update connection status cache", t);
402405
cachedNetworkCapabilities = null;
403-
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
406+
cacheFreshUntil = Deadline.after(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
404407
}
405408
}
406409
}
407410

408411
private boolean isCacheValid() {
409-
return (timeProvider.getCurrentTimeMillis() - lastCacheUpdateTime) < CACHE_TTL_MS;
412+
return !cacheFreshUntil.hasPassed();
410413
}
411414

412415
@Override
@@ -459,7 +462,7 @@ private void unregisterNetworkCallback(final boolean clearObservers) {
459462
// Clear cached state
460463
cachedNetworkCapabilities = null;
461464
currentNetwork = null;
462-
lastCacheUpdateTime = 0;
465+
cacheFreshUntil = Deadline.passed(clock);
463466
}
464467
options.getLogger().log(SentryLevel.DEBUG, "Network callback unregistered");
465468
}

‎sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import io.sentry.android.core.BuildInfoProvider
2626
import io.sentry.android.core.ContextUtils
2727
import io.sentry.android.core.SystemEventsBreadcrumbsIntegration
2828
import io.sentry.test.ImmediateExecutorService
29-
import io.sentry.transport.ICurrentDateProvider
29+
import io.sentry.time.TestTicker
30+
import java.util.concurrent.TimeUnit.MINUTES
3031
import kotlin.test.AfterTest
3132
import kotlin.test.BeforeTest
3233
import kotlin.test.Test
@@ -61,15 +62,13 @@ class AndroidConnectionStatusProviderTest {
6162
private lateinit var connectivityManager: ConnectivityManager
6263
private lateinit var networkInfo: NetworkInfo
6364
private lateinit var buildInfo: BuildInfoProvider
64-
private lateinit var timeProvider: ICurrentDateProvider
65+
private lateinit var clock: TestTicker
6566
private lateinit var options: SentryOptions
6667
private lateinit var network: Network
6768
private lateinit var networkCapabilities: NetworkCapabilities
6869
private lateinit var logger: ILogger
6970
private lateinit var contextUtilsStaticMock: MockedStatic<ContextUtils>
7071

71-
private var currentTime = 1000L
72-
7372
@BeforeTest
7473
fun beforeTest() {
7574
contextMock = mock()
@@ -96,17 +95,13 @@ class AndroidConnectionStatusProviderTest {
9695
whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true)
9796
whenever(networkCapabilities.hasTransport(TRANSPORT_WIFI)).thenReturn(true)
9897

99-
timeProvider = mock()
100-
whenever(timeProvider.currentTimeMillis).thenAnswer { currentTime }
98+
clock = TestTicker()
10199

102100
logger = mock()
103101
options = SentryOptions()
104102
options.setLogger(logger)
105103
options.executorService = ImmediateExecutorService()
106104

107-
// Reset current time for each test to ensure cache isolation
108-
currentTime = 1000L
109-
110105
// Mock ContextUtils to return foreground importance
111106
contextUtilsStaticMock = mockStatic(ContextUtils::class.java)
112107
contextUtilsStaticMock
@@ -120,7 +115,7 @@ class AndroidConnectionStatusProviderTest {
120115
AppState.getInstance().registerLifecycleObserver(options)
121116

122117
connectionStatusProvider =
123-
AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider)
118+
AndroidConnectionStatusProvider(contextMock, options, buildInfo, clock)
124119
}
125120

126121
@AfterTest
@@ -199,7 +194,7 @@ class AndroidConnectionStatusProviderTest {
199194

200195
// Create a new provider with the null connectivity manager
201196
val providerWithNullConnectivity =
202-
AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider)
197+
AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, clock)
203198

204199
assertEquals(
205200
IConnectionStatusProvider.ConnectionStatus.UNKNOWN,
@@ -310,6 +305,26 @@ class AndroidConnectionStatusProviderTest {
310305
assertTrue(connectionStatusProvider.statusObservers.isEmpty())
311306
}
312307

308+
@Test
309+
fun `an unpopulated cache is not treated as fresh shortly after boot`() {
310+
whenever(networkInfo.isConnected).thenReturn(true)
311+
312+
// elapsedRealtimeNanos() counts from boot, so a provider created moments after boot sees a
313+
// tick near zero. The cache is still empty and must not be read as up to date.
314+
val provider = AndroidConnectionStatusProvider(contextMock, options, buildInfo, TestTicker())
315+
316+
val callsBefore =
317+
mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" }
318+
319+
assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, provider.connectionStatus)
320+
321+
val callsAfter =
322+
mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" }
323+
assertTrue(callsAfter > callsBefore, "An empty cache must be populated before it is read")
324+
325+
provider.close()
326+
}
327+
313328
@Test
314329
fun `cache TTL works correctly`() {
315330
// Setup: Mock network info to return connected
@@ -327,7 +342,7 @@ class AndroidConnectionStatusProviderTest {
327342
mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" }
328343

329344
// Advance time by 1 minute (less than 2 minute TTL)
330-
currentTime += 60 * 1000L
345+
clock.advance(1, MINUTES)
331346

332347
// Second call should use cache - no additional calls to getActiveNetwork
333348
val secondResult = connectionStatusProvider.connectionStatus
@@ -340,7 +355,7 @@ class AndroidConnectionStatusProviderTest {
340355
assertEquals(initialCallCount, callCountAfterSecond, "Second call should use cache")
341356

342357
// Advance time beyond TTL (total 3 minutes)
343-
currentTime += 2 * 60 * 1000L
358+
clock.advance(2, MINUTES)
344359

345360
// Third call should refresh cache - should make new calls to getActiveNetwork
346361
val thirdResult = connectionStatusProvider.connectionStatus
@@ -547,7 +562,7 @@ class AndroidConnectionStatusProviderTest {
547562
whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(goodCaps)
548563

549564
// Force cache invalidation by advancing time beyond TTL
550-
currentTime += 3 * 60 * 1000L // 3 minutes
565+
clock.advance(3, MINUTES)
551566

552567
// Should return CONNECTED for good capabilities
553568
assertEquals(
@@ -564,7 +579,7 @@ class AndroidConnectionStatusProviderTest {
564579
whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(unvalidatedCaps)
565580

566581
// Force cache invalidation again
567-
currentTime += 3 * 60 * 1000L
582+
clock.advance(3, MINUTES)
568583

569584
assertEquals(
570585
IConnectionStatusProvider.ConnectionStatus.DISCONNECTED,

0 commit comments

Comments
 (0)