Skip to content

Commit 47f577f

Browse files
runningcodeclaude
andcommitted
fix(core): Clear updateRunning when a refresh cannot be queued
updateRunning is cleared in exactly one place, the submitted callable's finally block, so it is cleared if and only if the callable runs. Every failure from Future.get() leaves the callable running, so it still clears the flag itself. A failure from submit() does not: the callable was never queued, nothing clears the flag, and the compareAndSet guard in getHostname() then fails forever, so no refresh is ever attempted again. Removing MainEventProcessor's close() took away the only reachable way to make submit() throw, but the invariant was still wrong: a bounded queue, a shutdown added later, or a failure to start a thread would silently resurrect the same permanent freeze. Splitting submit() out of the try means the two cases can be told apart. Clearing the flag on a timeout or an interrupt as well would be wrong, since the callable is still running there and refreshes would pile up behind a slow lookup; MainEventProcessorTest's `sets servername to null if retrieving takes longer time` covers that path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1dbedb4 commit 47f577f

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.concurrent.ExecutorService;
99
import java.util.concurrent.Future;
1010
import java.util.concurrent.LinkedBlockingQueue;
11+
import java.util.concurrent.RejectedExecutionException;
1112
import java.util.concurrent.ThreadFactory;
1213
import java.util.concurrent.ThreadPoolExecutor;
1314
import java.util.concurrent.TimeUnit;
@@ -136,8 +137,21 @@ private void updateCache() {
136137
return null;
137138
};
138139

140+
final Future<Void> futureTask;
141+
try {
142+
futureTask = executorService.submit(hostRetriever);
143+
} catch (RejectedExecutionException e) {
144+
// updateRunning is cleared by the callable's finally block, which never runs if the callable
145+
// was never queued. Clearing it here keeps a failure to queue from latching the flag on and
146+
// silencing every later refresh.
147+
updateRunning.set(false);
148+
handleCacheUpdateFailure();
149+
return;
150+
}
151+
152+
// A timeout or interrupt below leaves the callable running, so it still clears updateRunning
153+
// itself; doing it here as well would let refreshes pile up behind a slow lookup.
139154
try {
140-
final Future<Void> futureTask = executorService.submit(hostRetriever);
141155
futureTask.get(GET_HOSTNAME_TIMEOUT, TimeUnit.MILLISECONDS);
142156
} catch (InterruptedException e) {
143157
Thread.currentThread().interrupt();

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

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

33
import com.google.common.truth.Truth.assertThat
44
import io.sentry.test.getProperty
5+
import io.sentry.test.injectForField
56
import java.net.InetAddress
67
import java.util.concurrent.ThreadPoolExecutor
78
import java.util.concurrent.TimeUnit
9+
import java.util.concurrent.atomic.AtomicBoolean
810
import kotlin.test.Test
911
import org.mockito.kotlin.mock
1012
import org.mockito.kotlin.whenever
@@ -23,6 +25,21 @@ class HostnameCacheTest {
2325
assertThat(cache.hostname).isEqualTo("myhost")
2426
}
2527

28+
@Test
29+
fun `a refresh that cannot be queued does not stop later refreshes`() {
30+
val cache = getSut()
31+
// Reject the next submit the way an executor that could not start a thread would, and mark the
32+
// cache stale so that reading the hostname attempts a refresh.
33+
cache.getProperty<ThreadPoolExecutor>("executorService").shutdown()
34+
cache.injectForField("expirationTimestamp", 0L)
35+
36+
assertThat(cache.hostname).isEqualTo("myhost")
37+
38+
// The callable never ran, so nothing else clears this flag; left set, it would fail the
39+
// compareAndSet guard in getHostname() and no refresh would ever be attempted again.
40+
assertThat(cache.getProperty<AtomicBoolean>("updateRunning").get()).isFalse()
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)