-
-
Notifications
You must be signed in to change notification settings - Fork 472
Fix leaked persisting executor in BaseCaptureStrategy #5564
Description
Each ReplayIntegration.start() creates a new SessionCaptureStrategy or BufferCaptureStrategy. Each strategy owns a lazy persistingExecutor in BaseCaptureStrategy, but stop() never shuts it down. When stop() runs on the main thread, it resets delegated properties via persistableAtomic*, which initializes the executor — leaving one leaked SentryReplayPersister-* thread per start/stop cycle.
Discovered via Kiln Android benchmarks after kiln#73 changed replay from a single long session to repeated start/stop loops: memory and CPU climbed monotonically, GC allocation rate for sdk-full reached ×ばつ baseline.
Root cause: BaseCaptureStrategy.stop() initializes but never shuts down the persistingExecutor on main-thread stop paths.
Recommended fix (PR1):
- Reuse the executor across stop/start cycles, or perform a non-blocking shutdown after stop-time property resets.
- Do not call
ReplayExecutorService.shutdown()on the stop path — it blocks and risks ANRs. - Store the raw delegate or an explicit lazy holder so shutdown only runs if the executor was initialized.
Follow-up:
- Add a regression test asserting no growth in
SentryReplayPersister-*threads across many start/stop cycles. - Rerun Kiln charts after fix; if GC blocking persists, investigate async bitmap/surface cleanup separately.
Action taken on behalf of nelson.osacky.