Skip to content

Commit 43cdac0

Browse files
runningcodeclaude
andcommitted
ref(core): Measure the hostname cache TTL on a monotonic ticker (JAVA-579)
HostnameCache stored an absolute expiry built from currentTimeMillis and compared it against a fresh reading, so a device time change resized the 5h TTL: a backward step extended it by the size of the step, a forward step expired the cache early. It now holds a Deadline on a MonotonicTicker. The field also no longer starts at 0, which on a boot-relative ticker reads as freshly set rather than as unset, so nothing counts as cached before the first resolve. The TTL is not a serialized value, so this only changes when the SDK re-resolves the hostname. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 033ce52 commit 43cdac0

2 files changed

Lines changed: 46 additions & 13 deletions

File tree

‎sentry/src/main/java/io/sentry/HostnameCache.java‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package io.sentry;
22

3+
import io.sentry.time.Deadline;
4+
import io.sentry.time.JavaMonotonicTicker;
5+
import io.sentry.time.MonotonicTicker;
36
import io.sentry.util.AutoClosableReentrantLock;
47
import io.sentry.util.Objects;
58
import java.net.InetAddress;
@@ -42,14 +45,16 @@ public final class HostnameCache {
4245
private static final @NotNull AutoClosableReentrantLock staticLock =
4346
new AutoClosableReentrantLock();
4447

45-
/** Time for which the cache is kept. */
46-
private final long cacheDuration;
48+
/** Time for which the cache is kept, in milliseconds. */
49+
private final long cacheDurationMillis;
50+
51+
private final @NotNull MonotonicTicker ticker;
4752

4853
/** Current value for hostname (might change over time). */
4954
@Nullable private volatile String hostname;
5055

51-
/** Time at which the cache should expire. */
52-
private volatile long expirationTimestamp;
56+
/** When the cached hostname goes stale. */
57+
private volatile @NotNull Deadline cacheFreshUntil;
5358

5459
/** Whether a cache update thread is currently running or not. */
5560
private final @NotNull AtomicBoolean updateRunning = new AtomicBoolean(false);
@@ -74,22 +79,34 @@ private HostnameCache() {
7479
this(HOSTNAME_CACHE_DURATION);
7580
}
7681

77-
HostnameCache(long cacheDuration) {
82+
HostnameCache(long cacheDurationMillis) {
7883
// avoid method refs on Android due to some issues with older AGP setups
7984
// noinspection Convert2MethodRef
80-
this(cacheDuration, () -> InetAddress.getLocalHost());
85+
this(cacheDurationMillis, () -> InetAddress.getLocalHost());
86+
}
87+
88+
HostnameCache(long cacheDurationMillis, final @NotNull Callable<InetAddress> getLocalhost) {
89+
this(cacheDurationMillis, getLocalhost, JavaMonotonicTicker.getInstance());
8190
}
8291

8392
/**
8493
* Sets up a cache for the hostname.
8594
*
86-
* @param cacheDuration cache duration in milliseconds.
95+
* @param cacheDurationMillis cache duration in milliseconds.
8796
* @param getLocalhost a callback to obtain the localhost address - this is mostly here because of
8897
* testability
98+
* @param ticker the ticker the cache lifetime is measured on
8999
*/
90-
HostnameCache(long cacheDuration, final @NotNull Callable<InetAddress> getLocalhost) {
91-
this.cacheDuration = cacheDuration;
100+
HostnameCache(
101+
long cacheDurationMillis,
102+
final @NotNull Callable<InetAddress> getLocalhost,
103+
final @NotNull MonotonicTicker ticker) {
104+
this.cacheDurationMillis = cacheDurationMillis;
92105
this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
106+
this.ticker = Objects.requireNonNull(ticker, "ticker is required");
107+
// Nothing resolved yet, so the cache is stale rather than fresh until updateCache says
108+
// otherwise.
109+
this.cacheFreshUntil = Deadline.passed(ticker);
93110
// A single thread executor whose worker thread times out while idle, so no thread is kept
94111
// alive between the infrequent cache refreshes.
95112
final @NotNull ThreadPoolExecutor executor =
@@ -122,8 +139,7 @@ boolean isClosed() {
122139
*/
123140
@Nullable
124141
public String getHostname() {
125-
if (expirationTimestamp < System.currentTimeMillis()
126-
&& updateRunning.compareAndSet(false, true)) {
142+
if (cacheFreshUntil.hasPassed() && updateRunning.compareAndSet(false, true)) {
127143
updateCache();
128144
}
129145

@@ -136,7 +152,7 @@ private void updateCache() {
136152
() -> {
137153
try {
138154
hostname = getLocalhost.call().getCanonicalHostName();
139-
expirationTimestamp = System.currentTimeMillis() + cacheDuration;
155+
cacheFreshUntil = Deadline.after(ticker, cacheDurationMillis, TimeUnit.MILLISECONDS);
140156
} finally {
141157
updateRunning.set(false);
142158
}
@@ -156,7 +172,7 @@ private void updateCache() {
156172
}
157173

158174
private void handleCacheUpdateFailure() {
159-
expirationTimestamp = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(1);
175+
cacheFreshUntil = Deadline.after(ticker, 1, TimeUnit.SECONDS);
160176
}
161177

162178
private static final class HostnameCacheThreadFactory implements ThreadFactory {

‎sentry/src/test/java/io/sentry/HostnameCacheTest.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package io.sentry
22

33
import com.google.common.truth.Truth.assertThat
44
import io.sentry.test.getProperty
5+
import io.sentry.time.TestMonotonicTicker
56
import java.net.InetAddress
67
import java.util.concurrent.ThreadPoolExecutor
78
import java.util.concurrent.TimeUnit
@@ -23,6 +24,22 @@ class HostnameCacheTest {
2324
assertThat(cache.hostname).isEqualTo("myhost")
2425
}
2526

27+
@Test
28+
fun `hostname is re-resolved only once the cache duration has elapsed`() {
29+
val ticker = TestMonotonicTicker()
30+
val address = mock<InetAddress>()
31+
whenever(address.canonicalHostName).thenReturn("first", "second")
32+
val cache = HostnameCache(TimeUnit.HOURS.toMillis(5), { address }, ticker)
33+
34+
assertThat(cache.hostname).isEqualTo("first")
35+
36+
ticker.advance(4, TimeUnit.HOURS)
37+
assertThat(cache.hostname).isEqualTo("first")
38+
39+
ticker.advance(1, TimeUnit.HOURS)
40+
assertThat(cache.hostname).isEqualTo("second")
41+
}
42+
2643
@Test
2744
fun `worker thread times out while idle instead of staying alive`() {
2845
val cache = getSut()

0 commit comments

Comments
 (0)