From 52761fd05487c09f9517df447a51ef3c45bb31d2 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 15 Sep 2026 16:44:35 +0200 Subject: [PATCH 1/4] ref(core): Hold the hostname cache on SentryOptions (JAVA-579) HostnameCache was a process-wide static singleton whose constructor reached for JavaMonotonicTicker.getInstance() directly. That was the last place in the SDK hard-coding a ticker rather than taking one from options, so the cache measured its TTL on System.nanoTime() even where options supply an elapsedRealtimeNanos()-backed ticker that counts deep sleep. SentryOptions now holds one instance, built from getMonotonicTicker() the same way RateLimiter(SentryOptions) is. It is wrapped in a LazyEvaluator because resolving the hostname blocks on InetAddress.getLocalHost(), which no Sentry.init should pay for up front; deferring also means the SentryAndroidOptions override is in effect by the time the ticker is read. MainEventProcessor still closes the cache, and now drops both its own reference and the options-held one. Both it and the options outlive Scopes.close(isRestarting = true), so a closed cache left in either place would never refresh the hostname again. MainEventProcessorTest no longer needs Mockito.mockStatic to intercept getInstance(); it sets the cache on options instead. Co-Authored-By: Claude Opus 5 (1M context) --- sentry/api/sentry.api | 4 +- .../main/java/io/sentry/HostnameCache.java | 26 ++-------- .../java/io/sentry/MainEventProcessor.java | 6 ++- .../main/java/io/sentry/SentryOptions.java | 33 ++++++++++++ .../main/java/io/sentry/logger/LoggerApi.java | 3 +- .../java/io/sentry/metrics/MetricsApi.java | 3 +- .../java/io/sentry/MainEventProcessorTest.kt | 22 +++----- .../test/java/io/sentry/SentryOptionsTest.kt | 52 +++++++++++++++++++ 8 files changed, 106 insertions(+), 43 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 24635fc5ddd..7d2e28b3a55 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -633,7 +633,6 @@ public final class io/sentry/Hint { public final class io/sentry/HostnameCache { public fun getHostname ()Ljava/lang/String; - public static fun getInstance ()Lio/sentry/HostnameCache; } public final class io/sentry/HttpStatusCodeRange { @@ -3716,6 +3715,7 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun getFlushTimeoutMillis ()J public fun getFullyDisplayedReporter ()Lio/sentry/FullyDisplayedReporter; public fun getGestureTargetLocators ()Ljava/util/List; + public fun getHostnameCache ()Lio/sentry/HostnameCache; public fun getIdleTimeout ()Ljava/lang/Long; public fun getIgnoredCheckIns ()Ljava/util/List; public fun getIgnoredErrors ()Ljava/util/List; @@ -3827,6 +3827,7 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun isTraceSampling ()Z public fun isTracingEnabled ()Z public fun merge (Lio/sentry/ExternalOptions;)V + public fun resetHostnameCache ()V public fun setAppStartExtender (Lio/sentry/IAppStartExtender;)V public fun setAttachServerName (Z)V public fun setAttachStacktrace (Z)V @@ -3887,6 +3888,7 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun setFullyDisplayedReporter (Lio/sentry/FullyDisplayedReporter;)V public fun setGestureTargetLocators (Ljava/util/List;)V public fun setGlobalHubMode (Ljava/lang/Boolean;)V + public fun setHostnameCache (Lio/sentry/HostnameCache;)V public fun setIdleTimeout (Ljava/lang/Long;)V public fun setIgnoredCheckIns (Ljava/util/List;)V public fun setIgnoredErrors (Ljava/util/List;)V diff --git a/sentry/src/main/java/io/sentry/HostnameCache.java b/sentry/src/main/java/io/sentry/HostnameCache.java index a5ae15258c6..47c44c370e0 100644 --- a/sentry/src/main/java/io/sentry/HostnameCache.java +++ b/sentry/src/main/java/io/sentry/HostnameCache.java @@ -1,9 +1,7 @@ package io.sentry; import io.sentry.time.Deadline; -import io.sentry.time.JavaMonotonicTicker; import io.sentry.time.MonotonicTicker; -import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; import java.net.InetAddress; import java.util.concurrent.Callable; @@ -28,8 +26,8 @@ * performance purposes, the operation of retrieving the hostname will automatically fail after a * period of time defined by {@link #GET_HOSTNAME_TIMEOUT} without result. * - *

HostnameCache is a singleton and its instance should be obtained through {@link - * HostnameCache#getInstance()}. + *

One instance is held per {@link SentryOptions} and should be obtained through {@link + * SentryOptions#getHostnameCache()}. */ @ApiStatus.Internal public final class HostnameCache { @@ -41,10 +39,6 @@ public final class HostnameCache { /** How long the worker thread may stay idle before it self-terminates. */ private static final long THREAD_KEEP_ALIVE_SECONDS = 30; - private static volatile @Nullable HostnameCache INSTANCE; - private static final @NotNull AutoClosableReentrantLock staticLock = - new AutoClosableReentrantLock(); - private final @NotNull MonotonicTicker ticker; /** Current value for hostname (might change over time). */ @@ -60,22 +54,10 @@ public final class HostnameCache { private final @NotNull ExecutorService executorService; - public static @NotNull HostnameCache getInstance() { - if (INSTANCE == null) { - try (final @NotNull ISentryLifecycleToken ignored = staticLock.acquire()) { - if (INSTANCE == null) { - INSTANCE = new HostnameCache(); - } - } - } - - return INSTANCE; - } - - private HostnameCache() { + HostnameCache(final @NotNull SentryOptions options) { // avoid method refs on Android due to some issues with older AGP setups // noinspection Convert2MethodRef - this(() -> InetAddress.getLocalHost(), JavaMonotonicTicker.getInstance()); + this(() -> InetAddress.getLocalHost(), options.getMonotonicTicker()); } /** diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index d84c9e47be8..90f3624da13 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -172,7 +172,7 @@ private void setServerName(final @NotNull SentryBaseEvent event) { private void ensureHostnameCache() { if (hostnameCache == null) { - hostnameCache = HostnameCache.getInstance(); + hostnameCache = options.getHostnameCache(); } } @@ -275,6 +275,10 @@ private boolean isCachedHint(final @NotNull Hint hint) { public void close() throws IOException { if (hostnameCache != null) { hostnameCache.close(); + // Both this processor and the options outlive a restart, so a closed cache left in either + // place would never refresh the hostname again. + hostnameCache = null; + options.resetHostnameCache(); } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 280bc01174e..ca5b9135128 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -530,6 +530,17 @@ public class SentryOptions implements RateLimiterConfig { private final @NotNull LazyEvaluator dateProvider = new LazyEvaluator<>(() -> new SentryAutoDateProvider()); + /** + * Cache of the local hostname, used when {@link #isAttachServerName()} is enabled. + * + *

Evaluated lazily because resolving the hostname blocks on {@code + * InetAddress.getLocalHost()}, which no {@code Sentry.init} should pay for up front. Deferring + * also means {@link #getMonotonicTicker()} is read after subclasses have overridden it. + */ + @ApiStatus.Internal + private final @NotNull LazyEvaluator hostnameCache = + new LazyEvaluator<>(() -> new HostnameCache(this)); + private final @NotNull List performanceCollectors = new ArrayList<>(); /** Performance collector that collect performance stats while transactions run. */ @@ -3092,6 +3103,28 @@ public void setDateProvider(final @NotNull SentryDateProvider dateProvider) { return JavaMonotonicTicker.getInstance(); } + /** Returns the hostname cache, resolving the hostname on first use. */ + @ApiStatus.Internal + public @NotNull HostnameCache getHostnameCache() { + return hostnameCache.getValue(); + } + + @ApiStatus.Internal + public void setHostnameCache(final @NotNull HostnameCache hostnameCache) { + this.hostnameCache.setValue(hostnameCache); + } + + /** + * Discards the cached instance, so that the next {@link #getHostnameCache()} builds a new one. + * + *

Called after the cache has been closed, because these options outlive a restart and a closed + * cache can no longer resolve anything. + */ + @ApiStatus.Internal + public void resetHostnameCache() { + hostnameCache.resetValue(); + } + /** * Adds a ICollector. * diff --git a/sentry/src/main/java/io/sentry/logger/LoggerApi.java b/sentry/src/main/java/io/sentry/logger/LoggerApi.java index c203dcbfb8f..ccf69325d86 100644 --- a/sentry/src/main/java/io/sentry/logger/LoggerApi.java +++ b/sentry/src/main/java/io/sentry/logger/LoggerApi.java @@ -1,6 +1,5 @@ package io.sentry.logger; -import io.sentry.HostnameCache; import io.sentry.IScope; import io.sentry.ISpan; import io.sentry.PropagationContext; @@ -263,7 +262,7 @@ private void setServerName( "server.address", new SentryLogEventAttributeValue(SentryAttributeType.STRING, optionsServerName)); } else if (options.isAttachServerName()) { - final @Nullable String hostname = HostnameCache.getInstance().getHostname(); + final @Nullable String hostname = options.getHostnameCache().getHostname(); if (hostname != null) { attributes.put( "server.address", diff --git a/sentry/src/main/java/io/sentry/metrics/MetricsApi.java b/sentry/src/main/java/io/sentry/metrics/MetricsApi.java index cebcad9735c..4cab828b7e3 100644 --- a/sentry/src/main/java/io/sentry/metrics/MetricsApi.java +++ b/sentry/src/main/java/io/sentry/metrics/MetricsApi.java @@ -1,6 +1,5 @@ package io.sentry.metrics; -import io.sentry.HostnameCache; import io.sentry.IScope; import io.sentry.ISpan; import io.sentry.PropagationContext; @@ -250,7 +249,7 @@ private void setServerName( "server.address", new SentryLogEventAttributeValue(SentryAttributeType.STRING, optionsServerName)); } else if (options.isAttachServerName()) { - final @Nullable String hostname = HostnameCache.getInstance().getHostname(); + final @Nullable String hostname = options.getHostnameCache().getHostname(); if (hostname != null) { attributes.put( "server.address", diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index ee88f06ae21..a77d2f76b92 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -11,7 +11,6 @@ import io.sentry.util.HintUtils import java.lang.RuntimeException import java.net.InetAddress import java.util.concurrent.TimeUnit -import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -19,7 +18,6 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue -import org.mockito.Mockito import org.mockito.kotlin.mock import org.mockito.kotlin.reset import org.mockito.kotlin.times @@ -39,7 +37,6 @@ class MainEventProcessorTest { val getLocalhost = mock() val hostnameCacheTicker = TestMonotonicTicker() lateinit var sentryTracer: SentryTracer - private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java) fun getSut( attachThreads: Boolean = true, @@ -77,20 +74,10 @@ class MainEventProcessorTest { whenever(scopes.options).thenReturn(sentryOptions) sentryTracer = SentryTracer(TransactionContext("", ""), scopes) - val hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker) - hostnameCacheMock.`when` { HostnameCache.getInstance() }.thenReturn(hostnameCache) + sentryOptions.setHostnameCache(HostnameCache({ getLocalhost }, hostnameCacheTicker)) return MainEventProcessor(sentryOptions) } - - fun teardown() { - hostnameCacheMock.close() - } - } - - @AfterTest - fun teardown() { - fixture.teardown() } private val fixture = Fixture() @@ -576,9 +563,14 @@ class MainEventProcessorTest { val sut = fixture.getSut(serverName = null) sut.process(SentryTransaction(fixture.sentryTracer), Hint()) + val hostnameCache = assertNotNull(sut.hostnameCache) sut.close() - assertNotNull(sut.hostnameCache) { assertTrue(it.isClosed) } + + assertTrue(hostnameCache.isClosed) + // Dropped on close so that a restart resolves the hostname again instead of reusing a cache + // whose executor is shut down. + assertNull(sut.hostnameCache) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 64482b5d5d0..9dedc9049f5 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -1,10 +1,17 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import io.sentry.SentryOptions.RequestSize import io.sentry.logger.ILoggerBatchProcessorFactory +import io.sentry.test.getProperty +import io.sentry.time.MonotonicTicker +import io.sentry.time.TestMonotonicTicker +import io.sentry.util.LazyEvaluator import io.sentry.util.StringUtils import java.io.File +import java.net.InetAddress import java.net.Proxy +import java.util.concurrent.Callable import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -1167,4 +1174,49 @@ class SentryOptionsTest { options.scopesStorageFactory = null assertNull(options.scopesStorageFactory) } + + private fun SentryOptions.peekHostnameCache(): HostnameCache? = + getProperty>("hostnameCache").getProperty("value") + + @Test + fun `constructing options does not resolve the hostname`() { + val options = SentryOptions() + assertThat(options.peekHostnameCache()).isNull() + + // Asserting that peek reports an evaluated cache keeps the check above from passing vacuously. + options.hostnameCache = HostnameCache(Callable { mock() }, TestMonotonicTicker()) + assertThat(options.peekHostnameCache()).isNotNull() + } + + @Test + fun `hostnameCache is created once and reused`() { + val options = SentryOptions() + val cache = HostnameCache(Callable { mock() }, TestMonotonicTicker()) + options.hostnameCache = cache + + assertThat(options.hostnameCache).isSameInstanceAs(cache) + assertThat(options.hostnameCache).isSameInstanceAs(cache) + } + + @Test + fun `resetHostnameCache discards the cached instance`() { + val options = SentryOptions() + options.hostnameCache = HostnameCache(Callable { mock() }, TestMonotonicTicker()) + + options.resetHostnameCache() + + assertThat(options.peekHostnameCache()).isNull() + } + + @Test + fun `hostnameCache measures its lifetime on the options ticker`() { + val ticker = TestMonotonicTicker() + val options = + object : SentryOptions() { + override fun getMonotonicTicker(): MonotonicTicker = ticker + } + + assertThat(options.hostnameCache.getProperty("ticker")) + .isSameInstanceAs(ticker) + } } From c94d2f6cba40926077f437ec10c74fba2da6e43e Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 15 Sep 2026 16:45:47 +0200 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e0d8bd82d2..3b62d13f3c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ - Deprecate `AndroidCurrentDateProvider.getInstance()` in favor of `MonotonicTicker`, which counts time spent in deep sleep and cannot be confused with the epoch-based `CurrentDateProvider` ([#6103](https://github.com/getsentry/sentry-java/pull/6103)) - Measure the hostname cache TTL on a monotonic ticker, so that a device time change no longer shortens or extends it ([#6100](https://github.com/getsentry/sentry-java/pull/6100)) +- Hold the hostname cache on `SentryOptions` instead of a static singleton, so that it measures its TTL on the ticker the options provide ([#6117](https://github.com/getsentry/sentry-java/pull/6117)) ## 8.56.0 From a4aecfed034d10a32cb23f2707028a741f1cfb56 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 15 Sep 2026 17:02:29 +0200 Subject: [PATCH 3/4] ref(core): Build the hostname cache from a ticker, not the whole options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HostnameCache reads exactly one collaborator, so it now takes a MonotonicTicker rather than a SentryOptions it would only call getMonotonicTicker() on. This follows RateLimiter.create(MonotonicTicker, RateLimiterConfig), which names the collaborators a rate limiter actually reads for the same reason; a config interface is unnecessary here because there is only the one. Also drops SentryOptions.setHostnameCache(), which had no production caller. Unlike setDateProvider(), which AndroidOptionsInitializer uses, it was only a test seam, so MainEventProcessorTest overrides getHostnameCache() instead — the way CheckInUtilsTest already overrides getMonotonicTicker(). Co-Authored-By: Claude Opus 5 (1M context) --- sentry/api/sentry.api | 1 - .../main/java/io/sentry/HostnameCache.java | 10 +++- .../main/java/io/sentry/SentryOptions.java | 7 +-- .../java/io/sentry/MainEventProcessorTest.kt | 24 ++++++---- .../test/java/io/sentry/SentryOptionsTest.kt | 46 +++++++------------ 5 files changed, 41 insertions(+), 47 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 7d2e28b3a55..309a38f71cd 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3888,7 +3888,6 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun setFullyDisplayedReporter (Lio/sentry/FullyDisplayedReporter;)V public fun setGestureTargetLocators (Ljava/util/List;)V public fun setGlobalHubMode (Ljava/lang/Boolean;)V - public fun setHostnameCache (Lio/sentry/HostnameCache;)V public fun setIdleTimeout (Ljava/lang/Long;)V public fun setIgnoredCheckIns (Ljava/util/List;)V public fun setIgnoredErrors (Ljava/util/List;)V diff --git a/sentry/src/main/java/io/sentry/HostnameCache.java b/sentry/src/main/java/io/sentry/HostnameCache.java index 47c44c370e0..23d9efad66b 100644 --- a/sentry/src/main/java/io/sentry/HostnameCache.java +++ b/sentry/src/main/java/io/sentry/HostnameCache.java @@ -54,10 +54,16 @@ public final class HostnameCache { private final @NotNull ExecutorService executorService; - HostnameCache(final @NotNull SentryOptions options) { + /** + * Names the only collaborator a hostname cache reads, rather than taking the whole {@link + * SentryOptions}. + * + * @param ticker the ticker the cache lifetime is measured on + */ + HostnameCache(final @NotNull MonotonicTicker ticker) { // avoid method refs on Android due to some issues with older AGP setups // noinspection Convert2MethodRef - this(() -> InetAddress.getLocalHost(), options.getMonotonicTicker()); + this(() -> InetAddress.getLocalHost(), ticker); } /** diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index ca5b9135128..8bc46d3ebf7 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -539,7 +539,7 @@ public class SentryOptions implements RateLimiterConfig { */ @ApiStatus.Internal private final @NotNull LazyEvaluator hostnameCache = - new LazyEvaluator<>(() -> new HostnameCache(this)); + new LazyEvaluator<>(() -> new HostnameCache(getMonotonicTicker())); private final @NotNull List performanceCollectors = new ArrayList<>(); @@ -3109,11 +3109,6 @@ public void setDateProvider(final @NotNull SentryDateProvider dateProvider) { return hostnameCache.getValue(); } - @ApiStatus.Internal - public void setHostnameCache(final @NotNull HostnameCache hostnameCache) { - this.hostnameCache.setValue(hostnameCache); - } - /** * Discards the cached instance, so that the next {@link #getHostnameCache()} builds a new one. * diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index a77d2f76b92..17eed1e01ed 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -26,16 +26,23 @@ import org.mockito.kotlin.whenever class MainEventProcessorTest { class Fixture { - val sentryOptions: SentryOptions = - SentryOptions().apply { - dsn = dsnString - release = "release" - dist = "dist" - sdkVersion = SdkVersion("test", "1.2.3") - } val scopes = mock() val getLocalhost = mock() val hostnameCacheTicker = TestMonotonicTicker() + // Built in getSut() rather than here: the constructor resolves the hostname straight away, + // so it has to run after getLocalhost is stubbed. + lateinit var hostnameCache: HostnameCache + val sentryOptions: SentryOptions = + object : SentryOptions() { + // Qualified: an unqualified name here would resolve to this override, not the field. + override fun getHostnameCache(): HostnameCache = this@Fixture.hostnameCache + } + .apply { + dsn = dsnString + release = "release" + dist = "dist" + sdkVersion = SdkVersion("test", "1.2.3") + } lateinit var sentryTracer: SentryTracer fun getSut( @@ -73,8 +80,7 @@ class MainEventProcessorTest { } whenever(scopes.options).thenReturn(sentryOptions) sentryTracer = SentryTracer(TransactionContext("", ""), scopes) - - sentryOptions.setHostnameCache(HostnameCache({ getLocalhost }, hostnameCacheTicker)) + hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker) return MainEventProcessor(sentryOptions) } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 9dedc9049f5..2aa23df9e17 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -9,9 +9,7 @@ import io.sentry.time.TestMonotonicTicker import io.sentry.util.LazyEvaluator import io.sentry.util.StringUtils import java.io.File -import java.net.InetAddress import java.net.Proxy -import java.util.concurrent.Callable import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -1178,45 +1176,35 @@ class SentryOptionsTest { private fun SentryOptions.peekHostnameCache(): HostnameCache? = getProperty>("hostnameCache").getProperty("value") + /** Options whose hostname cache resolves against a ticker the test controls. */ + private fun optionsWithTicker(ticker: MonotonicTicker): SentryOptions = + object : SentryOptions() { + override fun getMonotonicTicker(): MonotonicTicker = ticker + } + @Test - fun `constructing options does not resolve the hostname`() { - val options = SentryOptions() - assertThat(options.peekHostnameCache()).isNull() + fun `hostname is resolved on first use, not when options are constructed`() { + val ticker = TestMonotonicTicker() + val options = optionsWithTicker(ticker) - // Asserting that peek reports an evaluated cache keeps the check above from passing vacuously. - options.hostnameCache = HostnameCache(Callable { mock() }, TestMonotonicTicker()) - assertThat(options.peekHostnameCache()).isNotNull() - } + assertThat(options.peekHostnameCache()).isNull() - @Test - fun `hostnameCache is created once and reused`() { - val options = SentryOptions() - val cache = HostnameCache(Callable { mock() }, TestMonotonicTicker()) - options.hostnameCache = cache + val cache = options.hostnameCache + // Also keeps the assertion above honest: peek does report a cache once one exists. + assertThat(options.peekHostnameCache()).isSameInstanceAs(cache) assertThat(options.hostnameCache).isSameInstanceAs(cache) - assertThat(options.hostnameCache).isSameInstanceAs(cache) + assertThat(cache.getProperty("ticker")).isSameInstanceAs(ticker) } @Test fun `resetHostnameCache discards the cached instance`() { - val options = SentryOptions() - options.hostnameCache = HostnameCache(Callable { mock() }, TestMonotonicTicker()) + val options = optionsWithTicker(TestMonotonicTicker()) + val cache = options.hostnameCache options.resetHostnameCache() assertThat(options.peekHostnameCache()).isNull() - } - - @Test - fun `hostnameCache measures its lifetime on the options ticker`() { - val ticker = TestMonotonicTicker() - val options = - object : SentryOptions() { - override fun getMonotonicTicker(): MonotonicTicker = ticker - } - - assertThat(options.hostnameCache.getProperty("ticker")) - .isSameInstanceAs(ticker) + assertThat(options.hostnameCache).isNotSameInstanceAs(cache) } } From 71a7399cfa45b21ced966a86c663b8634133363e Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 15 Sep 2026 17:23:04 +0200 Subject: [PATCH 4/4] ref(core): Stop closing the hostname cache The cache's executor is a single daemon thread with allowCoreThreadTimeOut(true) and a 30 second keep-alive, so it self-terminates once idle and never holds up process exit. close() was switching off something that switches itself off. Scopes.close() already leaves the timer executor running for this reason. Dropping it removes the reason SentryOptions.resetHostnameCache() existed. That method was only there because MainEventProcessor closed a cache the options own, which a re-init with the same options object would then keep handing out shut down. Nothing closes the cache now, so nothing has to undo it. MainEventProcessor no longer implements Closeable, and no longer memoizes the cache either: the options build it on first use, so the field and ensureHostnameCache() were duplicating LazyEvaluator. Both removed methods are on @ApiStatus.Internal types. SentryClientTest's `when client is closed, hostname cache is closed` went with them; it asserted isClosed() on a processor that had never resolved a hostname, which returned true because the cache was still null. Co-Authored-By: Claude Opus 5 (1M context) --- sentry/api/sentry.api | 4 +- .../main/java/io/sentry/HostnameCache.java | 10 +---- .../java/io/sentry/MainEventProcessor.java | 42 +------------------ .../main/java/io/sentry/SentryOptions.java | 11 ----- .../test/java/io/sentry/HostnameCacheTest.kt | 7 ---- .../java/io/sentry/MainEventProcessorTest.kt | 15 ------- .../test/java/io/sentry/SentryClientTest.kt | 10 ----- .../test/java/io/sentry/SentryOptionsTest.kt | 11 ----- 8 files changed, 4 insertions(+), 106 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 309a38f71cd..d070668f03f 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1391,9 +1391,8 @@ public abstract interface class io/sentry/JsonUnknown { public abstract fun setUnknown (Ljava/util/Map;)V } -public final class io/sentry/MainEventProcessor : io/sentry/EventProcessor, java/io/Closeable { +public final class io/sentry/MainEventProcessor : io/sentry/EventProcessor { public fun (Lio/sentry/SentryOptions;)V - public fun close ()V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; public fun process (Lio/sentry/SentryLogEvent;)Lio/sentry/SentryLogEvent; @@ -3827,7 +3826,6 @@ public class io/sentry/SentryOptions : io/sentry/transport/RateLimiterConfig { public fun isTraceSampling ()Z public fun isTracingEnabled ()Z public fun merge (Lio/sentry/ExternalOptions;)V - public fun resetHostnameCache ()V public fun setAppStartExtender (Lio/sentry/IAppStartExtender;)V public fun setAttachServerName (Z)V public fun setAttachStacktrace (Z)V diff --git a/sentry/src/main/java/io/sentry/HostnameCache.java b/sentry/src/main/java/io/sentry/HostnameCache.java index 23d9efad66b..e32b319930c 100644 --- a/sentry/src/main/java/io/sentry/HostnameCache.java +++ b/sentry/src/main/java/io/sentry/HostnameCache.java @@ -81,7 +81,7 @@ public final class HostnameCache { // otherwise. this.cacheFreshUntil = Deadline.passed(ticker); // A single thread executor whose worker thread times out while idle, so no thread is kept - // alive between the infrequent cache refreshes. + // alive between the infrequent cache refreshes and nothing has to shut it down. final @NotNull ThreadPoolExecutor executor = new ThreadPoolExecutor( 1, @@ -95,14 +95,6 @@ public final class HostnameCache { updateCache(); } - void close() { - this.executorService.shutdown(); - } - - boolean isClosed() { - return this.executorService.isShutdown(); - } - /** * Gets the hostname of the current machine. * diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 90f3624da13..57a3a2e1c9c 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -8,23 +8,19 @@ import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.util.HintUtils; -import java.io.Closeable; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jetbrains.annotations.VisibleForTesting; @ApiStatus.Internal -public final class MainEventProcessor implements EventProcessor, Closeable { +public final class MainEventProcessor implements EventProcessor { private final @NotNull SentryOptions options; private final @NotNull SentryThreadFactory sentryThreadFactory; private final @NotNull SentryExceptionFactory sentryExceptionFactory; - private volatile @Nullable HostnameCache hostnameCache = null; public MainEventProcessor(final @NotNull SentryOptions options) { this.options = options; @@ -163,16 +159,7 @@ private void setServerName(final @NotNull SentryBaseEvent event) { } if (options.isAttachServerName() && event.getServerName() == null) { - ensureHostnameCache(); - if (hostnameCache != null) { - event.setServerName(hostnameCache.getHostname()); - } - } - } - - private void ensureHostnameCache() { - if (hostnameCache == null) { - hostnameCache = options.getHostnameCache(); + event.setServerName(options.getHostnameCache().getHostname()); } } @@ -271,31 +258,6 @@ private boolean isCachedHint(final @NotNull Hint hint) { return HintUtils.hasType(hint, Cached.class); } - @Override - public void close() throws IOException { - if (hostnameCache != null) { - hostnameCache.close(); - // Both this processor and the options outlive a restart, so a closed cache left in either - // place would never refresh the hostname again. - hostnameCache = null; - options.resetHostnameCache(); - } - } - - boolean isClosed() { - if (hostnameCache != null) { - return hostnameCache.isClosed(); - } else { - return true; - } - } - - @VisibleForTesting - @Nullable - HostnameCache getHostnameCache() { - return hostnameCache; - } - @Override public @Nullable Long getOrder() { return 0L; diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 8bc46d3ebf7..ebdb0b9011f 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -3109,17 +3109,6 @@ public void setDateProvider(final @NotNull SentryDateProvider dateProvider) { return hostnameCache.getValue(); } - /** - * Discards the cached instance, so that the next {@link #getHostnameCache()} builds a new one. - * - *

Called after the cache has been closed, because these options outlive a restart and a closed - * cache can no longer resolve anything. - */ - @ApiStatus.Internal - public void resetHostnameCache() { - hostnameCache.resetValue(); - } - /** * Adds a ICollector. * diff --git a/sentry/src/test/java/io/sentry/HostnameCacheTest.kt b/sentry/src/test/java/io/sentry/HostnameCacheTest.kt index 93cb9936962..6a87c32a0ae 100644 --- a/sentry/src/test/java/io/sentry/HostnameCacheTest.kt +++ b/sentry/src/test/java/io/sentry/HostnameCacheTest.kt @@ -48,11 +48,4 @@ class HostnameCacheTest { assertThat(executorService.corePoolSize).isEqualTo(1) assertThat(executorService.maximumPoolSize).isEqualTo(1) } - - @Test - fun `close shuts the executor down`() { - val cache = getSut() - cache.close() - assertThat(cache.isClosed).isTrue() - } } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 17eed1e01ed..9a02d9f4233 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -564,21 +564,6 @@ class MainEventProcessorTest { } } - @Test - fun `when processor is closed, closes hostname cache`() { - val sut = fixture.getSut(serverName = null) - - sut.process(SentryTransaction(fixture.sentryTracer), Hint()) - val hostnameCache = assertNotNull(sut.hostnameCache) - - sut.close() - - assertTrue(hostnameCache.isClosed) - // Dropped on close so that a restart resolves the hostname again instead of reusing a cache - // whose executor is shut down. - assertNull(sut.hostnameCache) - } - @Test fun `when event has modules, appends to them`() { val sut = fixture.getSut(modules = mapOf("group1:artifact1" to "2.0.0")) diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 3066d2c1b53..61181ee96a6 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -214,16 +214,6 @@ class SentryClientTest { assertFalse(sut.isEnabled) } - @Test - fun `when client is closed, hostname cache is closed`() { - val sut = fixture.getSut() - assertTrue(sut.isEnabled) - sut.close() - val mainEventProcessor = - fixture.sentryOptions.eventProcessors.filterIsInstance().first() - assertTrue(mainEventProcessor.isClosed) - } - @Test fun `when beforeSend is set, callback is invoked`() { var invoked = false diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 2aa23df9e17..f0bc051e984 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -1196,15 +1196,4 @@ class SentryOptionsTest { assertThat(options.hostnameCache).isSameInstanceAs(cache) assertThat(cache.getProperty("ticker")).isSameInstanceAs(ticker) } - - @Test - fun `resetHostnameCache discards the cached instance`() { - val options = optionsWithTicker(TestMonotonicTicker()) - val cache = options.hostnameCache - - options.resetHostnameCache() - - assertThat(options.peekHostnameCache()).isNull() - assertThat(options.hostnameCache).isNotSameInstanceAs(cache) - } }