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 2 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 @@ -10,13 +10,18 @@
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;

@Override
Expand Down Expand Up @@ -68,4 +73,9 @@ public void setApplicationContext(final @NotNull ApplicationContext applicationC
throws BeansException {
this.applicationContext = applicationContext;
}

@Override
public void destroy() {
Sentry.close();
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
}
}
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