From 3f176a7b687b4c0920d34f3b4ff0e2b24ce75fdc Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:36:55 -0700 Subject: [PATCH 1/2] fix: guard executor shutdown in BaseCaptureStrategy.stop() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each start/stop cycle leaked one SentryReplayPersister-* thread because stop() reset delegated properties (segmentTimestamp, currentReplayId) whose setters dispatch to persistingExecutor, initialising the lazy — but stop() never shut it down. Replace the lazy delegate with an explicit nullable holder so the executor is only created when actually needed and can be detected at stop() time. Call shutdownNow() (non-blocking) rather than the blocking shutdown() to avoid ANRs when stop() runs on the main thread. Fixes #5564 --- .../replay/capture/BaseCaptureStrategy.kt | 26 ++++++++++++--- .../capture/SessionCaptureStrategyTest.kt | 32 +++++++++++++++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index dab98ec4e2..280ffe24d1 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -58,11 +58,20 @@ internal abstract class BaseCaptureStrategy( private const val MAX_TRACE_IDS = 100 } - private val persistingExecutor: ScheduledExecutorService by lazy { - val delegate = - Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory()) - ReplayExecutorService(delegate, options) - } + // Explicit holder so we can detect whether the executor was ever initialised and shut it down + // without initialising it as a side-effect (which is what a plain `lazy` delegate would do). + private var persistingExecutorHolder: ScheduledExecutorService? = null + private val persistingExecutor: ScheduledExecutorService + get() { + return persistingExecutorHolder + ?: run { + val delegate = + Executors.newSingleThreadScheduledExecutor( + ReplayPersistingExecutorServiceThreadFactory() + ) + ReplayExecutorService(delegate, options).also { persistingExecutorHolder = it } + } + } private val gestureConverter = ReplayGestureConverter(dateProvider) protected val isTerminating = AtomicBoolean(false) @@ -123,6 +132,13 @@ internal abstract class BaseCaptureStrategy( replayStartTimestamp.set(0) segmentTimestamp = null currentReplayId = SentryId.EMPTY_ID + // Shut down the persisting executor only if it was actually initialised. We must not call the + // blocking ReplayExecutorService.shutdown() here because stop() may run on the main thread, + // and blocking there risks an ANR. shutdownNow() is non-blocking: it cancels queued tasks and + // interrupts any running one, which is acceptable because the tasks are best-effort persistence + // writes and the cache has already been closed above. + persistingExecutorHolder?.shutdownNow() + persistingExecutorHolder = null } protected fun createSegmentInternal( diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index b5a00bc624..570753c3a6 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -7,6 +7,7 @@ import io.sentry.IScopes import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions +import io.sentry.util.thread.IThreadChecker import io.sentry.SentryReplayEvent import io.sentry.SentryReplayEvent.ReplayType import io.sentry.SentryReplayOptions.SentryReplayQuality.HIGH @@ -554,4 +555,35 @@ class SessionCaptureStrategyTest { any(), ) } + + @Test + fun `stop shuts down persisting executor so no SentryReplayPersister threads leak across cycles`() { + // Force isMainThread() to return true so that property-reset writes in stop() are dispatched + // to the persistingExecutor, which is the code path that used to leak threads. + fixture.options.threadChecker = + mock { + on { isMainThread() }.thenReturn(true) + on { isMainThread(any()) }.thenReturn(true) + on { isMainThread(anyLong()) }.thenReturn(false) + on { currentThreadName }.thenReturn("main") + on { currentThreadSystemId() }.thenReturn(0L) + } + + repeat(3) { + val strategy = fixture.getSut() + strategy.start(0, SentryId()) + strategy.onConfigurationChanged(fixture.recorderConfig) + strategy.stop() + } + + // Allow time for thread termination after shutdownNow(). + Thread.sleep(200) + + val leakedThreads = + Thread.getAllStackTraces().keys.filter { it.name.startsWith("SentryReplayPersister-") } + assertTrue( + leakedThreads.isEmpty(), + "Expected no SentryReplayPersister threads after stop(), found: ${leakedThreads.map { it.name }}", + ) + } } From 5b032f29f85bc8d9b58831e76aa5bc1836105f25 Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Thu, 25 Jun 2026 19:03:53 -0700 Subject: [PATCH 2/2] style: apply spotless formatting --- .../io/sentry/android/replay/capture/BaseCaptureStrategy.kt | 1 + .../sentry/android/replay/capture/SessionCaptureStrategyTest.kt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt index 280ffe24d1..bacf672291 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt @@ -72,6 +72,7 @@ internal abstract class BaseCaptureStrategy( ReplayExecutorService(delegate, options).also { persistingExecutorHolder = it } } } + private val gestureConverter = ReplayGestureConverter(dateProvider) protected val isTerminating = AtomicBoolean(false) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index 570753c3a6..6343ca6f42 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -7,7 +7,6 @@ import io.sentry.IScopes import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions -import io.sentry.util.thread.IThreadChecker import io.sentry.SentryReplayEvent import io.sentry.SentryReplayEvent.ReplayType import io.sentry.SentryReplayOptions.SentryReplayQuality.HIGH @@ -32,6 +31,7 @@ import io.sentry.rrweb.RRWebMetaEvent import io.sentry.rrweb.RRWebOptionsEvent import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider +import io.sentry.util.thread.IThreadChecker import java.io.File import java.util.Date import kotlin.test.Test