Skip to content

Commit

Permalink
Ensure Hints do not cause memory leaks (#2387)
Browse files Browse the repository at this point in the history
Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com>
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
  • Loading branch information
3 people committed Nov 28, 2022
1 parent b39d6a0 commit 451f2fe
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@
- Use `canonicalName` in Fragment Integration for better de-obfuscation ([#2379](https://github.com/getsentry/sentry-java/pull/2379))
- Fix Timber and Fragment integrations auto-installation for obfuscated builds ([#2379](https://github.com/getsentry/sentry-java/pull/2379))
- Don't attach screenshots to events from Hybrid SDKs ([#2360](https://github.com/getsentry/sentry-java/pull/2360))
- Ensure Hints do not cause memory leaks ([#2387](https://github.com/getsentry/sentry-java/pull/2387))

### Dependencies

Expand Down
1 change: 1 addition & 0 deletions sentry/api/sentry.api
Expand Up @@ -249,6 +249,7 @@ public final class io/sentry/Hint {
public fun <init> ()V
public fun addAttachment (Lio/sentry/Attachment;)V
public fun addAttachments (Ljava/util/List;)V
public fun clear ()V
public fun clearAttachments ()V
public fun get (Ljava/lang/String;)Ljava/lang/Object;
public fun getAs (Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object;
Expand Down
28 changes: 24 additions & 4 deletions sentry/src/main/java/io/sentry/Hint.java
Expand Up @@ -2,8 +2,10 @@

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -39,16 +41,17 @@ public final class Hint {
return hint;
}

public void set(@NotNull String name, @Nullable Object hint) {
public synchronized void set(@NotNull String name, @Nullable Object hint) {
internalStorage.put(name, hint);
}

public @Nullable Object get(@NotNull String name) {
public synchronized @Nullable Object get(@NotNull String name) {
return internalStorage.get(name);
}

@SuppressWarnings("unchecked")
public <T extends Object> @Nullable T getAs(@NotNull String name, @NotNull Class<T> clazz) {
public synchronized <T extends Object> @Nullable T getAs(
@NotNull String name, @NotNull Class<T> clazz) {
Object hintValue = internalStorage.get(name);

if (clazz.isInstance(hintValue)) {
Expand All @@ -60,7 +63,7 @@ public void set(@NotNull String name, @Nullable Object hint) {
}
}

public void remove(@NotNull String name) {
public synchronized void remove(@NotNull String name) {
internalStorage.remove(name);
}

Expand Down Expand Up @@ -89,6 +92,23 @@ public void clearAttachments() {
attachments.clear();
}

/**
* Clears all attributes added via {@link #set(String, Object)} Note: SDK internal attributes are
* being kept. This is useful to avoid leaking any objects (e.g. Android activities) being
* referenced.
*/
@ApiStatus.Internal
public synchronized void clear() {
final Iterator<Map.Entry<String, Object>> iterator = internalStorage.entrySet().iterator();

while (iterator.hasNext()) {
final Map.Entry<String, Object> entry = iterator.next();
if (entry.getKey() == null || !entry.getKey().startsWith("sentry:")) {
iterator.remove();
}
}
}

public void setScreenshot(@Nullable Attachment screenshot) {
this.screenshot = screenshot;
}
Expand Down
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/SentryClient.java
Expand Up @@ -178,6 +178,7 @@ private boolean shouldApplyScopeData(
final SentryEnvelope envelope =
buildEnvelope(event, attachments, session, traceContext, null);

hint.clear();
if (envelope != null) {
transport.send(envelope, hint);
}
Expand Down Expand Up @@ -483,6 +484,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
}

try {
hint.clear();
transport.send(envelope, hint);
} catch (IOException e) {
options.getLogger().log(SentryLevel.ERROR, "Failed to capture envelope.", e);
Expand Down Expand Up @@ -588,6 +590,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
buildEnvelope(
transaction, filterForTransaction(getAttachments(hint)), null, traceContext, null);

hint.clear();
if (envelope != null) {
transport.send(envelope, hint);
} else {
Expand Down
20 changes: 20 additions & 0 deletions sentry/src/test/java/io/sentry/hints/HintTest.kt
@@ -1,7 +1,9 @@
package io.sentry.hints

import io.sentry.Attachment
import io.sentry.CachedEvent
import io.sentry.Hint
import io.sentry.TypeCheckHint.SENTRY_TYPE_CHECK_HINT
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
Expand Down Expand Up @@ -197,6 +199,24 @@ class HintTest {
assertEquals(listOf(attachment3, attachment4), hint.attachments)
}

@Test
fun `calling clear does not remove internal sentry attrs, attachments or screenshots`() {
val userAttribute = "app_label"

val hint = Hint()
hint.set(SENTRY_TYPE_CHECK_HINT, CachedEvent())
hint.set(userAttribute, "test label")
hint.addAttachment(newAttachment("test attachment"))
hint.screenshot = newAttachment("2")

hint.clear()

assertNotNull(hint.get(SENTRY_TYPE_CHECK_HINT))
assertNull(hint.get(userAttribute))
assertEquals(1, hint.attachments.size)
assertNotNull(hint.screenshot)
}

companion object {
fun newAttachment(content: String) = Attachment(content.toByteArray(), "$content.txt")
}
Expand Down

0 comments on commit 451f2fe

Please sign in to comment.