Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,14 @@ public final class io/sentry/android/core/MemoryLimiterIntegration : io/sentry/I
public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
}

public final class io/sentry/android/core/MemoryLimiterIntegration$MemoryLimiterHint : io/sentry/hints/BlockingFlushHint, io/sentry/hints/Backfillable {
public final class io/sentry/android/core/MemoryLimiterIntegration$MemoryLimiterHint : io/sentry/hints/BlockingFlushHint, io/sentry/hints/AbnormalExit, io/sentry/hints/Backfillable {
public fun <init> (JLio/sentry/ILogger;JZ)V
public fun ignoreCurrentThread ()Z
public fun isFlushable (Lio/sentry/protocol/SentryId;)Z
public fun mechanism ()Ljava/lang/String;
public fun setFlushable (Lio/sentry/protocol/SentryId;)V
public fun shouldEnrich ()Z
public fun timestamp ()J
public fun timestamp ()Ljava/lang/Long;
}

public final class io/sentry/android/core/NativeEventCollector {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,8 +587,6 @@ private void setDist(
timestamp = ((AbnormalExit) hint).timestamp();
} else if (hint instanceof NativeCrashExit) {
timestamp = ((NativeCrashExit) hint).timestamp();
} else if (hint instanceof MemoryLimiterIntegration.MemoryLimiterHint) {
timestamp = ((MemoryLimiterIntegration.MemoryLimiterHint) hint).timestamp();
} else {
timestamp = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.sentry.SentryOptions;
import io.sentry.android.core.ApplicationExitInfoHistoryDispatcher.ApplicationExitInfoPolicy;
import io.sentry.android.core.cache.AndroidEnvelopeCache;
import io.sentry.hints.AbnormalExit;
import io.sentry.hints.Backfillable;
import io.sentry.hints.BlockingFlushHint;
import io.sentry.protocol.Mechanism;
Expand Down Expand Up @@ -76,6 +77,7 @@ public final class MemoryLimiterIntegration implements Integration, Closeable {
static final @NotNull String MEMORY_LIMITER_DESCRIPTION =
MEMORY_LIMITER_DESCRIPTION_PREFIX + "AnonSwap";

static final @NotNull String MEMORY_LIMITER_MECHANISM = "memory_limiter";
static final @NotNull String MEMORY_LIMITER_MESSAGE = "Android process killed by MemoryLimiter";

static final @NotNull String IMPORTANCE_DATA_KEY = "importance";
Expand Down Expand Up @@ -318,7 +320,8 @@ public void markReported(final long timestamp) {
* backfilled with persisted launch state or kept as a lighter historical record.
*/
@ApiStatus.Internal
public static final class MemoryLimiterHint extends BlockingFlushHint implements Backfillable {
public static final class MemoryLimiterHint extends BlockingFlushHint
implements Backfillable, AbnormalExit {

private final long timestamp;
private final boolean shouldEnrich;
Expand All @@ -333,10 +336,21 @@ public MemoryLimiterHint(
this.shouldEnrich = shouldEnrich;
}

public long timestamp() {
@Override
public @NotNull Long timestamp() {
return timestamp;
}

@Override
public @NotNull String mechanism() {
return MEMORY_LIMITER_MECHANISM;
}

@Override
public boolean ignoreCurrentThread() {
return false;
}
Comment thread
0xadam-brown marked this conversation as resolved.

@Override
public boolean shouldEnrich() {
return shouldEnrich;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,10 @@ class MemoryLimiterIntegrationTest {
},
argThat<Hint> {
val hint = HintUtils.getSentrySdkHint(this) as MemoryLimiterHint
hint.shouldEnrich() && hint.timestamp() == newTimestamp
hint.shouldEnrich() &&
hint.timestamp() == newTimestamp &&
hint.mechanism() == MemoryLimiterIntegration.MEMORY_LIMITER_MECHANISM &&
!hint.ignoreCurrentThread()
},
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
package io.sentry.android.core.cache

import com.google.common.truth.Truth.assertThat
import io.sentry.DateUtils
import io.sentry.ISerializer
import io.sentry.NoOpLogger
import io.sentry.SentryEnvelope
import io.sentry.SentryEvent
import io.sentry.SentryOptions
import io.sentry.SentryUUID
import io.sentry.Session
import io.sentry.Session.State.Abnormal
import io.sentry.Session.State.Ok
import io.sentry.UncaughtExceptionHandlerIntegration.UncaughtExceptionHint
import io.sentry.android.core.AnrV2Integration.AnrV2Hint
import io.sentry.android.core.MemoryLimiterIntegration.MemoryLimiterHint
Expand All @@ -14,6 +21,7 @@ import io.sentry.transport.ICurrentDateProvider
import io.sentry.util.HintUtils
import java.io.File
import java.lang.IllegalArgumentException
import java.util.Date
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
Expand Down Expand Up @@ -227,6 +235,51 @@ class AndroidEnvelopeCacheTest {
assertEquals("23456789", fixture.lastReportedMemoryLimiterFile.readText())
}

@Test
fun `memory limiter hint marks previous session abnormal at exit timestamp`() {
val cache = fixture.getSut(tmpDir)
val sessionStart = DateUtils.getCurrentDateTime()
val exitTimestamp = sessionStart.time + 1_000
val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
fixture.options.serializer.serialize(createSession(sessionStart), previousSessionFile.writer())
val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null)

cache.storeEnvelope(
envelope,
HintUtils.createWithTypeCheckHint(
MemoryLimiterHint(0, NoOpLogger.getInstance(), exitTimestamp, true)
),
)

val updatedSession =
fixture.options.serializer.deserialize(previousSessionFile.reader(), Session::class.java)!!
assertThat(updatedSession.status).isEqualTo(Abnormal)
assertThat(updatedSession.timestamp!!.time).isEqualTo(exitTimestamp)
assertThat(updatedSession.abnormalMechanism).isEqualTo("memory_limiter")
}

// Protects against misbehaved clocks, stale cached state, or mismatched recovery data.
@Test
fun `memory limiter exit before previous session start does not mark session abnormal`() {
val cache = fixture.getSut(tmpDir)
val sessionStart = DateUtils.getCurrentDateTime()
val previousSessionFile = EnvelopeCache.getPreviousSessionFile(fixture.options.cacheDirPath!!)
fixture.options.serializer.serialize(createSession(sessionStart), previousSessionFile.writer())
val envelope = SentryEnvelope.from(fixture.options.serializer, SentryEvent(), null)

cache.storeEnvelope(
envelope,
HintUtils.createWithTypeCheckHint(
MemoryLimiterHint(0, NoOpLogger.getInstance(), sessionStart.time - 1_000, true)
),
)

val updatedSession =
fixture.options.serializer.deserialize(previousSessionFile.reader(), Session::class.java)!!
assertThat(updatedSession.status).isEqualTo(Ok)
assertThat(updatedSession.abnormalMechanism).isNull()
}

@Test
fun `memory limiter and anr markers are stored independently`() {
val cache = fixture.getSut(tmpDir)
Expand Down Expand Up @@ -260,5 +313,23 @@ class AndroidEnvelopeCacheTest {
assertFalse(didStore)
}

private fun createSession(started: Date): Session =
Session(
Ok,
started,
started,
0,
"distinct-id",
SentryUUID.generateSentryId(),
true,
null,
null,
null,
null,
"environment",
"release",
null,
)

internal class UncaughtHint : UncaughtExceptionHint(0, NoOpLogger.getInstance())
}
26 changes: 19 additions & 7 deletions sentry/src/main/java/io/sentry/hints/AbnormalExit.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,34 @@
/**
* Marker interface for Sessions experiencing abnormal status.
*
* <p><b>Note:</b> While this interface applies to the broad category of abnormal exits (meaning any
* exits that weren't classified as normal terminations or crashes) it currently is exclusively used
* as a hint marker for Android ANRs (both watchdog and ApplicationExitInfo based). If additional
* categories of abnormal exits were introduced, all instances of discriminator code (`instanceof
* AbnormalExit`) should be carefully reviewed for ANR specifics accidentally being applied.
* <p>Includes exits that were not classified as normal terminations or crashes, such as Android
* ANRs and MemoryLimiter process deaths.
*
* <p><b>Note:</b> Some existing discriminator code (`instanceof AbnormalExit`) is shaped by the
* historical ANR-only usage of this interface. New implementations should review all of those call
* sites carefully to ensure ANR-specific behavior isn't applied accidentally.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Fyi, I checked those call sites in connection with the work here, and we didn't need to make any changes.)

*/
public interface AbnormalExit {

/** What was the mechanism this Session has abnormal'ed with */
@Nullable
String mechanism();

/** Whether the current thread should be ignored from being marked as crashed, e.g. a watchdog */
/**
* Whether the current thread (e.g., a watchdog) should be ignored by the `MainEventProcessor`
* when deciding which threads should from the current process should be bound to the Sentry event
* associated with this `AbnormalExit`.
*
* <p>This method effectively no-ops for types implementing both {@link AbnormalExit} and {@link
* Backfillable}, as implementors of `Backfillable` are not sent to the `MainEventProcessor`.
*/
boolean ignoreCurrentThread();

/** When exactly the abnormal exit happened */
/**
* When exactly the abnormal exit happened.
*
* <p>Epoch time in milliseconds, or null.
*/
@Nullable
Long timestamp();
}