Lazily retrieve HostnameCache in MainEventProcessor (#2170)

* Prevent ANR by HostnameCache * Add changelog
getsentry · Jul 14, 2022 · 8c459df · 8c459df
1 parent f160e0d
commit 8c459df
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Lazily retrieve HostnameCache in MainEventProcessor ([#2170](https://github.com/getsentry/sentry-java/pull/2170))
+
 ## 6.2.1
 
 ### Fixes

diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java
@@ -31,16 +31,10 @@ public final class MainEventProcessor implements EventProcessor, Closeable {
   private final @NotNull SentryOptions options;
   private final @NotNull SentryThreadFactory sentryThreadFactory;
   private final @NotNull SentryExceptionFactory sentryExceptionFactory;
-  private final @Nullable HostnameCache hostnameCache;
+  private volatile @Nullable HostnameCache hostnameCache = null;
 
   public MainEventProcessor(final @NotNull SentryOptions options) {
-    this(options, options.isAttachServerName() ? HostnameCache.getInstance() : null);
-  }
-
-  MainEventProcessor(
-      final @NotNull SentryOptions options, final @Nullable HostnameCache hostnameCache) {
     this.options = Objects.requireNonNull(options, "The SentryOptions is required.");
-    this.hostnameCache = hostnameCache;
 
     final SentryStackTraceFactory sentryStackTraceFactory =
         new SentryStackTraceFactory(
@@ -53,14 +47,12 @@ public MainEventProcessor(final @NotNull SentryOptions options) {
   MainEventProcessor(
       final @NotNull SentryOptions options,
       final @NotNull SentryThreadFactory sentryThreadFactory,
-      final @NotNull SentryExceptionFactory sentryExceptionFactory,
-      final @NotNull HostnameCache hostnameCache) {
+      final @NotNull SentryExceptionFactory sentryExceptionFactory) {
     this.options = Objects.requireNonNull(options, "The SentryOptions is required.");
     this.sentryThreadFactory =
         Objects.requireNonNull(sentryThreadFactory, "The SentryThreadFactory is required.");
     this.sentryExceptionFactory =
         Objects.requireNonNull(sentryExceptionFactory, "The SentryExceptionFactory is required.");
-    this.hostnameCache = Objects.requireNonNull(hostnameCache, "The HostnameCache is required");
   }
 
   @Override
@@ -164,8 +156,21 @@ private void setServerName(final @NotNull SentryBaseEvent event) {
       event.setServerName(options.getServerName());
     }
 
-    if (options.isAttachServerName() && hostnameCache != null && event.getServerName() == null) {
-      event.setServerName(hostnameCache.getHostname());
+    if (options.isAttachServerName() && event.getServerName() == null) {
+      ensureHostnameCache();
+      if (hostnameCache != null) {
+        event.setServerName(hostnameCache.getHostname());
+      }
+    }
+  }
+
+  private void ensureHostnameCache() {
+    if (hostnameCache == null) {
+      synchronized (this) {
+        if (hostnameCache == null) {
+          hostnameCache = HostnameCache.getInstance();
+        }
+      }
     }
   }
 

diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt
@@ -12,8 +12,10 @@ import io.sentry.protocol.SentryTransaction
 import io.sentry.protocol.User
 import io.sentry.util.HintUtils
 import org.awaitility.kotlin.await
+import org.mockito.Mockito
 import java.lang.RuntimeException
 import java.net.InetAddress
+import kotlin.test.AfterTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -32,10 +34,12 @@ class MainEventProcessorTest {
         }
         val getLocalhost = mock<InetAddress>()
         val sentryTracer = SentryTracer(TransactionContext("", ""), mock())
+        private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java)
 
         fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map<String, String> = emptyMap(), sendDefaultPii: Boolean? = null, serverName: String? = "server", host: String? = null, resolveHostDelay: Long? = null, hostnameCacheDuration: Long = 10, proguardUuid: String? = null): MainEventProcessor {
             sentryOptions.isAttachThreads = attachThreads
             sentryOptions.isAttachStacktrace = attachStackTrace
+            sentryOptions.isAttachServerName = true
             sentryOptions.environment = environment
             sentryOptions.serverName = serverName
             if (sendDefaultPii != null) {
@@ -52,10 +56,21 @@ class MainEventProcessorTest {
                 host
             }
             val hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost }
-            return MainEventProcessor(sentryOptions, hostnameCache)
+            hostnameCacheMock.`when`<Any> { HostnameCache.getInstance() }.thenReturn(hostnameCache)
+
+            return MainEventProcessor(sentryOptions)
+        }
+
+        fun teardown() {
+            hostnameCacheMock.close()
         }
     }
 
+    @AfterTest
+    fun teardown() {
+        fixture.teardown()
+    }
+
     private val fixture = Fixture()
 
     @Test
@@ -448,7 +463,10 @@ class MainEventProcessorTest {
 
     @Test
     fun `when processor is closed, closes hostname cache`() {
-        val sut = fixture.getSut()
+        val sut = fixture.getSut(serverName = null)
+
+        sut.process(SentryTransaction(fixture.sentryTracer), Hint())
+
         sut.close()
         assertNotNull(sut.hostnameCache) {
             assertTrue(it.isClosed())