Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential memory leaks. #1909

Merged
merged 3 commits into from Feb 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

/** 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();
}
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
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