fix(core): Keep resolving the hostname after Sentry.close()

CHANGELOG.md

@@ -34,6 +34,10 @@
 - Sentry can now configure Log4j2 automatically for Spring Boot 3 when `sentry-log4j2` is on the classpath and Log4j2 Core is the active logging backend ([#6072](https://github.com/getsentry/sentry-java/pull/6072))
   - Disabled by default for now; enable it and configure levels the same way as described in the Spring Boot 4 entry above (`sentry.logging.enabled=true`)
 
+### Fixes
+
+- Keep resolving the server name after `Sentry.close()` or a re-init. Closing the SDK shut down the shared hostname cache for the life of the process, so `server_name` silently froze at the value it had last resolved ([#6119](https://github.com/getsentry/sentry-java/pull/6119))
+
 ### Internal
 
 - 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))

sentry/api/sentry.api

@@ -1392,9 +1392,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 <init> (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;

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

@@ -8,6 +8,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
@@ -91,7 +92,7 @@ private HostnameCache() {
     this.cacheDuration = cacheDuration;
     this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
     // 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,
@@ -105,14 +106,6 @@ private HostnameCache() {
     updateCache();
   }
 
-  void close() {
-    this.executorService.shutdown();
-  }
-
-  boolean isClosed() {
-    return this.executorService.isShutdown();
-  }
-
   /**
    * Gets the hostname of the current machine.
    *
@@ -144,8 +137,21 @@ private void updateCache() {
           return null;
         };
 
+    final Future<Void> futureTask;
+    try {
+      futureTask = executorService.submit(hostRetriever);
+    } catch (RejectedExecutionException e) {
+      // updateRunning is cleared by the callable's finally block, which never runs if the callable
+      // was never queued. Clearing it here keeps a failure to queue from latching the flag on and
+      // silencing every later refresh.
+      updateRunning.set(false);
+      handleCacheUpdateFailure();
+      return;
+    }
+
+    // A timeout or interrupt below leaves the callable running, so it still clears updateRunning
+    // itself; doing it here as well would let refreshes pile up behind a slow lookup.
     try {
-      final Future<Void> futureTask = executorService.submit(hostRetriever);
       futureTask.get(GET_HOSTNAME_TIMEOUT, TimeUnit.MILLISECONDS);
     } catch (InterruptedException e) {
       Thread.currentThread().interrupt();

sentry/src/main/java/io/sentry/MainEventProcessor.java

@@ -8,18 +8,15 @@
 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;
@@ -271,27 +268,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();
-    }
-  }
-
-  boolean isClosed() {
-    if (hostnameCache != null) {
-      return hostnameCache.isClosed();
-    } else {
-      return true;
-    }
-  }
-
-  @VisibleForTesting
-  @Nullable
-  HostnameCache getHostnameCache() {
-    return hostnameCache;
-  }
-
   @Override
   public @Nullable Long getOrder() {
     return 0L;

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

@@ -2,9 +2,11 @@ package io.sentry
 
 import com.google.common.truth.Truth.assertThat
 import io.sentry.test.getProperty
+import io.sentry.test.injectForField
 import java.net.InetAddress
 import java.util.concurrent.ThreadPoolExecutor
 import java.util.concurrent.TimeUnit
+import java.util.concurrent.atomic.AtomicBoolean
 import kotlin.test.Test
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
@@ -23,6 +25,21 @@ class HostnameCacheTest {
     assertThat(cache.hostname).isEqualTo("myhost")
   }
 
+  @Test
+  fun `a refresh that cannot be queued does not stop later refreshes`() {
+    val cache = getSut()
+    // Reject the next submit the way an executor that could not start a thread would, and mark the
+    // cache stale so that reading the hostname attempts a refresh.
+    cache.getProperty<ThreadPoolExecutor>("executorService").shutdown()
+    cache.injectForField("expirationTimestamp", 0L)
+
+    assertThat(cache.hostname).isEqualTo("myhost")
+
+    // The callable never ran, so nothing else clears this flag; left set, it would fail the
+    // compareAndSet guard in getHostname() and no refresh would ever be attempted again.
+    assertThat(cache.getProperty<AtomicBoolean>("updateRunning").get()).isFalse()
+  }
+
   @Test
   fun `worker thread times out while idle instead of staying alive`() {
     val cache = getSut()
@@ -31,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()
-  }
 }

sentry/src/test/java/io/sentry/MainEventProcessorTest.kt

@@ -571,16 +571,6 @@ class MainEventProcessorTest {
     }
   }
 
-  @Test
-  fun `when processor is closed, closes hostname cache`() {
-    val sut = fixture.getSut(serverName = null)
-
-    sut.process(SentryTransaction(fixture.sentryTracer), Hint())
-
-    sut.close()
-    assertNotNull(sut.hostnameCache) { assertTrue(it.isClosed) }
-  }
-
   @Test
   fun `when event has modules, appends to them`() {
     val sut = fixture.getSut(modules = mapOf("group1:artifact1" to "2.0.0"))

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<MainEventProcessor>().first()
-    assertTrue(mainEventProcessor.isClosed)
-  }
-
   @Test
   fun `when beforeSend is set, callback is invoked`() {
     var invoked = false