From 6ee6a404825b6649f4c917665fa16d5c154a15ea 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/8] 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 8bef6abddcfe3d56a142d1f09b2dfa700d72c342 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/8] 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 From 856e99fa24683ffc88933df356ffc2fb47863090 Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Wed, 1 Jul 2026 07:08:49 -0700 Subject: [PATCH 3/8] refactor(replay): move persistingExecutor ownership to ReplayIntegration Move persistingExecutor out of BaseCaptureStrategy and into ReplayIntegration, passing it as a constructor argument to CaptureStrategy subclasses. Shut it down in ReplayIntegration.close() alongside replayExecutor so executor lifecycle is managed in one place. --- .../android/replay/ReplayIntegration.kt | 18 ++++++++++ .../replay/capture/BaseCaptureStrategy.kt | 36 +------------------ .../replay/capture/BufferCaptureStrategy.kt | 8 +++-- .../replay/capture/SessionCaptureStrategy.kt | 11 +++++- .../capture/BufferCaptureStrategyTest.kt | 6 ++++ .../capture/SessionCaptureStrategyTest.kt | 8 +++++ 6 files changed, 49 insertions(+), 38 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index 116ab45af0..67ec568266 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -111,6 +111,11 @@ public class ReplayIntegration( val delegate = Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) ReplayExecutorService(delegate, options) } + private val persistingExecutor by lazy { + val delegate = + Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory()) + ReplayExecutorService(delegate, options) + } internal val isEnabled = AtomicBoolean(false) internal val isManualPause = AtomicBoolean(false) @@ -192,6 +197,7 @@ public class ReplayIntegration( scopes, dateProvider, replayExecutor, + persistingExecutor, replayCacheProvider, ) } else { @@ -201,6 +207,7 @@ public class ReplayIntegration( dateProvider, random, replayExecutor, + persistingExecutor, replayCacheProvider, ) } @@ -374,6 +381,7 @@ public class ReplayIntegration( recorder = null rootViewsSpy.close() replayExecutor.shutdown() + persistingExecutor.shutdown() lifecycle.currentState = CLOSED } } @@ -554,4 +562,14 @@ public class ReplayIntegration( return ret } } + + private class ReplayPersistingExecutorServiceThreadFactory : ThreadFactory { + private var cnt = 0 + + override fun newThread(r: Runnable): Thread { + val ret = Thread(r, "SentryReplayPersister-" + cnt++) + ret.setDaemon(true) + return ret + } + } } 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 bacf672291..6bb58c5e2a 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 @@ -25,7 +25,6 @@ import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.Companion.createSegment import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.gestures.ReplayGestureConverter -import io.sentry.android.replay.util.ReplayExecutorService import io.sentry.android.replay.util.ReplayRunnable import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebEvent @@ -34,9 +33,7 @@ import java.io.File import java.util.Date import java.util.Deque import java.util.concurrent.ConcurrentLinkedDeque -import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ThreadFactory import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.atomic.AtomicReference @@ -50,6 +47,7 @@ internal abstract class BaseCaptureStrategy( private val scopes: IScopes?, private val dateProvider: ICurrentDateProvider, protected val replayExecutor: ScheduledExecutorService, + protected val persistingExecutor: ScheduledExecutorService, private val replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null, ) : CaptureStrategy { internal companion object { @@ -58,21 +56,6 @@ internal abstract class BaseCaptureStrategy( private const val MAX_TRACE_IDS = 100 } - // 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) @@ -133,13 +116,6 @@ 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( @@ -209,16 +185,6 @@ internal abstract class BaseCaptureStrategy( } } - private class ReplayPersistingExecutorServiceThreadFactory : ThreadFactory { - private var cnt = 0 - - override fun newThread(r: Runnable): Thread { - val ret = Thread(r, "SentryReplayPersister-" + cnt++) - ret.setDaemon(true) - return ret - } - } - private inline fun persistableAtomicNullable( initialValue: T? = null, propertyName: String, diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt index 0eea2043bd..0df8a642f6 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt @@ -33,6 +33,7 @@ internal class BufferCaptureStrategy( private val dateProvider: ICurrentDateProvider, private val random: Random, executor: ScheduledExecutorService, + persistingExecutor: ScheduledExecutorService, replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null, ) : BaseCaptureStrategy( @@ -40,6 +41,7 @@ internal class BufferCaptureStrategy( scopes, dateProvider, executor, + persistingExecutor, replayCacheProvider = replayCacheProvider, ) { // TODO: capture envelopes for buffered segments instead, but don't send them until buffer is @@ -150,8 +152,10 @@ internal class BufferCaptureStrategy( ) return this } - // we hand over replayExecutor to the new strategy to preserve order of execution - val captureStrategy = SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor) + // we hand over replayExecutor and persistingExecutor to the new strategy to preserve order of + // execution + val captureStrategy = + SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, persistingExecutor) captureStrategy.recorderConfig = recorderConfig captureStrategy.start( segmentId = currentSegment, diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index 4d3ee588f0..d62efb534c 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -21,8 +21,17 @@ internal class SessionCaptureStrategy( private val scopes: IScopes?, private val dateProvider: ICurrentDateProvider, executor: ScheduledExecutorService, + persistingExecutor: ScheduledExecutorService, replayCacheProvider: ((replayId: SentryId) -> ReplayCache)? = null, -) : BaseCaptureStrategy(options, scopes, dateProvider, executor, replayCacheProvider) { +) : + BaseCaptureStrategy( + options, + scopes, + dateProvider, + executor, + persistingExecutor, + replayCacheProvider, + ) { internal companion object { private const val TAG = "SessionCaptureStrategy" } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt index 380e9b3ce7..b5048e856f 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/BufferCaptureStrategyTest.kt @@ -111,6 +111,12 @@ class BufferCaptureStrategyTest { null } }, + mock { + whenever(it.submit(any())).doAnswer { invocation -> + (invocation.arguments[0] as Runnable).run() + null + } + }, ) { _ -> replayCache } 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 6343ca6f42..8d4c943b8e 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 @@ -123,6 +123,14 @@ class SessionCaptureStrategyTest { .whenever(it) .submit(any()) }, + mock { + doAnswer { invocation -> + (invocation.arguments[0] as Runnable).run() + null + } + .whenever(it) + .submit(any()) + }, ) { _ -> replayCache } From 6d4e33bfdfeefd755153cea9c3536800e523c304 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 1 Jul 2026 19:01:35 +0200 Subject: [PATCH 4/8] Fix leak in ReplayIntegration due to persisting executor not being shut down Add the persistingExecutor argument to SessionCaptureStrategy and BufferCaptureStrategy constructor calls in tests, and add changelog entry. Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 2 ++ .../sentry/android/replay/ReplayIntegrationTest.kt | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73bb2ef396..109f291299 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +<<<<<<< HEAD ### Behavioral Changes - `SentryOkHttpInterceptor::intercept` now throws `IOException`. This is a source-only and Java-only breaking change ([#5654](https://github.com/getsentry/sentry-java/pull/5654)) @@ -11,6 +12,7 @@ - Don't start a redundant UI interaction transaction when a transaction is already bound to the Scope ([#5491](https://github.com/getsentry/sentry-java/issues/5491)) - Previously, `SentryGestureListener` always started a UI transaction and only afterwards skipped binding it to the Scope when a manually-bound transaction already existed, leaving the new transaction to be dropped as an idle transaction without children. - Fix potential NPE within `Scope.endSession()` ([#5657](https://github.com/getsentry/sentry-java/pull/5657)) +- Fix memory leak in `ReplayIntegration` due to persisting executor not being shut down ([#5627](https://github.com/getsentry/sentry-java/pull/5627)) ### Performance diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index 3df0c9f005..898585b0ef 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -754,6 +754,12 @@ class ReplayIntegrationTest { null } }, + mock { + whenever(mock.submit(any())).doAnswer { + (it.arguments[0] as Runnable).run() + null + } + }, ) { _ -> fixture.replayCache } @@ -1116,5 +1122,12 @@ class ReplayIntegrationTest { null } }, + persistingExecutor = + mock { + whenever(mock.submit(any())).doAnswer { + (it.arguments[0] as Runnable).run() + null + } + }, ) } From d4887f4ea582019bbb8a9e10af1359d77ef5da69 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 1 Jul 2026 19:11:58 +0200 Subject: [PATCH 5/8] Remove stray merge conflict marker from CHANGELOG.md Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 109f291299..b2f260761b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ## Unreleased -<<<<<<< HEAD ### Behavioral Changes - `SentryOkHttpInterceptor::intercept` now throws `IOException`. This is a source-only and Java-only breaking change ([#5654](https://github.com/getsentry/sentry-java/pull/5654)) From bb4b7e0a682d247f3872876d5c27d5911bcccf27 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 1 Jul 2026 19:17:31 +0200 Subject: [PATCH 6/8] Remove no-op leak test from SessionCaptureStrategyTest The test used a mocked executor that never spawned threads, so the thread-count assertion was always true regardless of the fix. The executor lifecycle is now owned by ReplayIntegration, not SessionCaptureStrategy, so the test belonged at the wrong layer. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../capture/SessionCaptureStrategyTest.kt | 32 ------------------- 1 file changed, 32 deletions(-) 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 8d4c943b8e..dd9e6c6ce1 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 @@ -31,7 +31,6 @@ 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 @@ -563,35 +562,4 @@ 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 3b21f8f4a801728f19f54bfe9c1988cda5ad0361 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 1 Jul 2026 19:36:12 +0200 Subject: [PATCH 7/8] Add executor leak regression test to ReplayIntegrationTest Uses real ScheduledThreadPoolExecutor threads so the test actually fails if the shutdown in close() is removed. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../android/replay/ReplayIntegrationTest.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index 898585b0ef..2ff2392eba 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -1110,6 +1110,32 @@ class ReplayIntegrationTest { assertEquals(traceId, traceIdRegistered) } + @Test + fun `close shuts down persisting executor so no SentryReplayPersister threads leak`() { + fixture.options.cacheDirPath = tmpDir.newFolder().absolutePath + fixture.options.threadChecker = mock { + on { isMainThread }.thenReturn(true) + on { isMainThread(any()) }.thenReturn(true) + } + + repeat(3) { + val replay = fixture.getSut(context) + replay.register(fixture.scopes, fixture.options) + replay.start() + replay.stop() + replay.close() + } + + Thread.sleep(200) + + val leakedThreads = + Thread.getAllStackTraces().keys.filter { it.name.startsWith("SentryReplayPersister-") } + assertTrue( + leakedThreads.isEmpty(), + "Expected no SentryReplayPersister threads after close(), found: ${leakedThreads.map { it.name }}", + ) + } + private fun getSessionCaptureStrategy(options: SentryOptions): SessionCaptureStrategy = SessionCaptureStrategy( options, From c14175f2ee916c87a831b166723b32910e1babc0 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 1 Jul 2026 21:00:50 +0200 Subject: [PATCH 8/8] Use shutdownNow() for replay executors in close() to avoid ANR shutdown() calls awaitTermination() which blocks up to shutdownTimeoutMillis. Since close() can run on the main thread (via Sentry.close() from hybrid SDKs), this risks an ANR. shutdownNow() is non-blocking and sufficient at teardown. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../android/replay/ReplayIntegration.kt | 22 +++++++++++++++---- .../replay/util/ReplayExecutorService.kt | 8 +++++++ .../android/replay/ReplayIntegrationTest.kt | 13 ++++++----- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index 67ec568266..21b945766b 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -107,15 +107,17 @@ public class ReplayIntegration( private var gestureRecorder: GestureRecorder? = null private val random by lazy { Random() } internal val rootViewsSpy by lazy { RootViewsSpy.install() } - private val replayExecutor by lazy { + private val lazyReplayExecutor = lazy { val delegate = Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory()) ReplayExecutorService(delegate, options) } - private val persistingExecutor by lazy { + private val replayExecutor by lazyReplayExecutor + private val lazyPersistingExecutor = lazy { val delegate = Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory()) ReplayExecutorService(delegate, options) } + private val persistingExecutor by lazyPersistingExecutor internal val isEnabled = AtomicBoolean(false) internal val isManualPause = AtomicBoolean(false) @@ -380,8 +382,20 @@ public class ReplayIntegration( recorder?.close() recorder = null rootViewsSpy.close() - replayExecutor.shutdown() - persistingExecutor.shutdown() + if (lazyReplayExecutor.isInitialized()) { + if (options.threadChecker.isMainThread) { + replayExecutor.gracefulShutdown() + } else { + replayExecutor.shutdown() + } + } + if (lazyPersistingExecutor.isInitialized()) { + if (options.threadChecker.isMainThread) { + persistingExecutor.gracefulShutdown() + } else { + persistingExecutor.shutdown() + } + } lifecycle.currentState = CLOSED } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt index 31a3279d07..9e9491f516 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt @@ -57,6 +57,14 @@ internal class ReplayExecutorService( } } } + + fun gracefulShutdown() { + synchronized(this) { + if (!isShutdown) { + delegate.shutdown() + } + } + } } internal class ReplayRunnable(val taskName: String, delegate: Runnable) : Runnable by delegate diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt index 2ff2392eba..1c87701841 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayIntegrationTest.kt @@ -48,6 +48,7 @@ import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider import io.sentry.transport.RateLimiter import io.sentry.util.Random +import io.sentry.util.thread.IThreadChecker import java.io.ByteArrayOutputStream import java.io.File import kotlin.test.BeforeTest @@ -1113,21 +1114,21 @@ class ReplayIntegrationTest { @Test fun `close shuts down persisting executor so no SentryReplayPersister threads leak`() { fixture.options.cacheDirPath = tmpDir.newFolder().absolutePath - fixture.options.threadChecker = mock { - on { isMainThread }.thenReturn(true) - on { isMainThread(any()) }.thenReturn(true) - } + val mainThreadChecker = mock() + whenever(mainThreadChecker.isMainThread).thenReturn(true) + whenever(mainThreadChecker.isMainThread(any())).thenReturn(true) + val defaultThreadChecker = fixture.options.threadChecker repeat(3) { + fixture.options.threadChecker = mainThreadChecker val replay = fixture.getSut(context) replay.register(fixture.scopes, fixture.options) replay.start() replay.stop() + fixture.options.threadChecker = defaultThreadChecker replay.close() } - Thread.sleep(200) - val leakedThreads = Thread.getAllStackTraces().keys.filter { it.name.startsWith("SentryReplayPersister-") } assertTrue(