Skip to content

Commit

Permalink
Fix potential memory leaks. (#1909)
Browse files Browse the repository at this point in the history
  • Loading branch information
maciejwalkowiak committed Feb 14, 2022
1 parent 33bdce3 commit 651fa3d
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* Fix: Do not include stacktrace frames into Timber message (#1898)
* Fix: Potential memory leaks (#1909)

Breaking changes:
`Timber.tag` is no longer supported by our [Timber integration](https://docs.sentry.io/platforms/android/configuration/integrations/timber/) and will not appear on Sentry for error events.
Expand Down
3 changes: 2 additions & 1 deletion sentry-spring/api/sentry-spring.api
Expand Up @@ -27,8 +27,9 @@ public class io/sentry/spring/SentryHubRegistrar : org/springframework/context/a
public fun registerBeanDefinitions (Lorg/springframework/core/type/AnnotationMetadata;Lorg/springframework/beans/factory/support/BeanDefinitionRegistry;)V
}

public class io/sentry/spring/SentryInitBeanPostProcessor : org/springframework/beans/factory/config/BeanPostProcessor, org/springframework/context/ApplicationContextAware {
public class io/sentry/spring/SentryInitBeanPostProcessor : org/springframework/beans/factory/DisposableBean, org/springframework/beans/factory/config/BeanPostProcessor, org/springframework/context/ApplicationContextAware {
public fun <init> ()V
public fun destroy ()V
public fun postProcessAfterInitialization (Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object;
public fun setApplicationContext (Lorg/springframework/context/ApplicationContext;)V
}
Expand Down
Expand Up @@ -2,23 +2,42 @@

import com.jakewharton.nopen.annotation.Open;
import io.sentry.EventProcessor;
import io.sentry.HubAdapter;
import io.sentry.IHub;
import io.sentry.ITransportFactory;
import io.sentry.Integration;
import io.sentry.Sentry;
import io.sentry.SentryOptions;
import io.sentry.SentryOptions.TracesSamplerCallback;
import io.sentry.util.Objects;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.DisposableBean;
import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;

/** Initializes Sentry after all beans are registered. */
/**
* Initializes Sentry after all beans are registered. Closes Sentry on Spring application context
* destroy.
*/
@Open
public class SentryInitBeanPostProcessor implements BeanPostProcessor, ApplicationContextAware {
public class SentryInitBeanPostProcessor
implements BeanPostProcessor, ApplicationContextAware, DisposableBean {
private @Nullable ApplicationContext applicationContext;

private final @NotNull IHub hub;

public SentryInitBeanPostProcessor() {
this(HubAdapter.getInstance());
}

SentryInitBeanPostProcessor(final @NotNull IHub hub) {
Objects.requireNonNull(hub, "hub is required");
this.hub = hub;
}

@Override
@SuppressWarnings({"unchecked", "deprecation"})
public @NotNull Object postProcessAfterInitialization(
Expand Down Expand Up @@ -68,4 +87,9 @@ public void setApplicationContext(final @NotNull ApplicationContext applicationC
throws BeansException {
this.applicationContext = applicationContext;
}

@Override
public void destroy() {
hub.close();
}
}
@@ -0,0 +1,30 @@
package io.sentry.spring

import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import io.sentry.IHub
import org.springframework.context.annotation.AnnotationConfigApplicationContext
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import kotlin.test.Test

class SentryInitBeanPostProcessorTest {

@Test
fun closesSentryOnApplicationContextDestroy() {
val ctx = AnnotationConfigApplicationContext(TestConfig::class.java)
val hub = ctx.getBean(IHub::class.java)
ctx.close()
verify(hub).close()
}

@Configuration
open class TestConfig {

@Bean(destroyMethod = "")
open fun hub() = mock<IHub>()

@Bean
open fun sentryInitBeanPostProcessor() = SentryInitBeanPostProcessor(hub())
}
}
15 changes: 14 additions & 1 deletion sentry/src/main/java/io/sentry/HostnameCache.java
Expand Up @@ -21,12 +21,18 @@
* by keeping track of the hostname for a period defined during the construction. For performance
* purposes, the operation of retrieving the hostname will automatically fail after a period of time
* defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
*
* <p>HostnameCache is a singleton and its instance should be obtained through {@link
* HostnameCache#getInstance()}.
*/
final class HostnameCache {
private static final long HOSTNAME_CACHE_DURATION = TimeUnit.HOURS.toMillis(5);

/** Time before the get hostname operation times out (in ms). */
private static final long GET_HOSTNAME_TIMEOUT = TimeUnit.SECONDS.toMillis(1);

@Nullable private static HostnameCache INSTANCE;

/** Time for which the cache is kept. */
private final long cacheDuration;
/** Current value for hostname (might change over time). */
Expand All @@ -41,7 +47,14 @@ final class HostnameCache {
private final @NotNull ExecutorService executorService =
Executors.newSingleThreadExecutor(new HostnameCacheThreadFactory());

public HostnameCache() {
static @NotNull HostnameCache getInstance() {
if (INSTANCE == null) {
INSTANCE = new HostnameCache();
}
return INSTANCE;
}

private HostnameCache() {
this(HOSTNAME_CACHE_DURATION);
}

Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/MainEventProcessor.java
Expand Up @@ -34,7 +34,7 @@ public final class MainEventProcessor implements EventProcessor, Closeable {
private final @Nullable HostnameCache hostnameCache;

MainEventProcessor(final @NotNull SentryOptions options) {
this(options, options.isAttachServerName() ? new HostnameCache() : null);
this(options, options.isAttachServerName() ? HostnameCache.getInstance() : null);
}

MainEventProcessor(
Expand Down
2 changes: 2 additions & 0 deletions sentry/src/main/java/io/sentry/Sentry.java
Expand Up @@ -237,6 +237,8 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
public static synchronized void close() {
final IHub hub = getCurrentHub();
mainHub = NoOpHub.getInstance();
// remove thread local to avoid memory leak
currentHub.remove();
hub.close();
}

Expand Down

0 comments on commit 651fa3d

Please sign in to comment.