From 6b591a12abbd0b94e893d516ea210e857cd198d5 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 23 Jun 2026 14:41:59 +0200 Subject: [PATCH 1/5] fix(replay): Release MediaMuxer when the encoder fails to start The MediaMuxer is opened eagerly in SimpleVideoEncoder's constructor, but its release() was only reachable on paths that assume start() succeeded. Two cases leaked it: - createVideoOf constructed the encoder and called start() in one expression, so when start() threw the encoder was never assigned and release() could never run. - SimpleVideoEncoder.release() released the muxer as the last statement of the try block, after draining and stopping the codec. Draining a codec that never started throws, skipping the muxer release. Release the encoder if start() throws, and always release the muxer from a finally block so it is freed even when draining/stopping the codec fails. This surfaced as a CloseGuard "resource was acquired but never released" warning. Complements #5583. Co-Authored-By: Claude Opus 4.8 --- .../io/sentry/android/replay/ReplayCache.kt | 11 ++++++++- .../replay/video/SimpleVideoEncoder.kt | 8 ++++--- .../sentry/android/replay/ReplayCacheTest.kt | 23 +++++++++++++++++++ .../replay/util/ReplayShadowMediaCodec.kt | 4 ++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt index b54177bca9b..55891fea1a7 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt @@ -162,7 +162,16 @@ public class ReplayCache(private val options: SentryOptions, private val replayI bitRate = bitRate, ), ) - .also { it.start() } + .apply { + // the constructor already opened the MediaMuxer, so release it if start() fails, + // otherwise the encoder is never assigned and its resources leak (CloseGuard warning) + try { + start() + } catch (t: Throwable) { + release() + throw t + } + } } val step = 1000 / frameRate.toLong() diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 04d573d793f..8490bd80150 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -287,12 +287,14 @@ internal class SimpleVideoEncoder( onClose?.invoke() drainCodec(true) mediaCodec.stop() + } catch (e: RuntimeException) { + options.logger.log(DEBUG, "Failed to properly release video encoder", e) + } finally { + // always release the native resources, even if draining/stopping the codec above threw (e.g. + // when the encoder failed to fully start), otherwise they leak (CloseGuard warning) mediaCodec.release() surface?.release() - frameMuxer.release() - } catch (e: Throwable) { - options.logger.log(DEBUG, "Failed to properly release video encoder", e) } } } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt index 8b64ca5caeb..23e8d60044f 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt @@ -28,6 +28,7 @@ import java.util.concurrent.atomic.AtomicReference import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue @@ -36,6 +37,7 @@ import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowBitmapFactory +import org.robolectric.shadows.ShadowCloseGuard @RunWith(AndroidJUnit4::class) @Config(sdk = [26], shadows = [ReplayShadowMediaCodec::class]) @@ -56,6 +58,7 @@ class ReplayCacheTest { @BeforeTest fun `set up`() { ReplayShadowMediaCodec.framesToEncode = 5 + ReplayShadowMediaCodec.throwOnStart = false ShadowBitmapFactory.setAllowInvalidImageData(true) } @@ -93,6 +96,26 @@ class ReplayCacheTest { assertNull(video) } + @Test + fun `releases the muxer when the encoder fails to start`() { + ReplayShadowMediaCodec.throwOnStart = true + val replayCache = fixture.getSut(tmpDir) + + val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) + replayCache.addFrame(bitmap, 1) + + ShadowCloseGuard.reset() + assertFailsWith { + replayCache.createVideoOf(5000L, 0, 0, 100, 200, 1, 20_000) + } + + val muxerLeaks = + ShadowCloseGuard.getErrors().filter { error -> + error.stackTrace.any { it.className.contains("MediaMuxer") } + } + assertTrue(muxerLeaks.isEmpty(), "MediaMuxer was not released: $muxerLeaks") + } + @Test fun `deletes frames after creating a video`() { ReplayShadowMediaCodec.framesToEncode = 3 diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/util/ReplayShadowMediaCodec.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/util/ReplayShadowMediaCodec.kt index f60c6688386..60ccb157747 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/util/ReplayShadowMediaCodec.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/util/ReplayShadowMediaCodec.kt @@ -15,12 +15,16 @@ class ReplayShadowMediaCodec : ShadowMediaCodec() { companion object { var frameRate = 1 var framesToEncode = 5 + var throwOnStart = false } private val encoded = AtomicBoolean(false) @Implementation fun start() { + if (throwOnStart) { + throw IllegalStateException("Simulated codec start failure") + } super.native_start() } From 7cab63295529e8c8f4a321563aed83dd59e5d09e Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 23 Jun 2026 14:43:15 +0200 Subject: [PATCH 2/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f46c77b306c..edd2c8d174d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Pin the published Sentry Android SDK's AAR metadata `minCompileSdk` to our `minSdk` (`21`) instead of AGP 9's new default of the SDK's own `compileSdk` (`37`), so apps that depend on the SDK aren't forced to raise their `compileSdk` ([#5823](https://github.com/getsentry/sentry-java/pull/5823)) +- Release `MediaMuxer` when the replay video encoder fails to start to avoid a resource leak ([#5607](https://github.com/getsentry/sentry-java/pull/5607)) ### Performance From b38dd345e2120b83c89fa6f1069286619cdb31cb Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 23 Jul 2026 12:32:56 +0200 Subject: [PATCH 3/5] fix(replay): Skip MediaMuxer stop when no samples were written MediaMuxer.stop() throws IllegalStateException when the muxer was started but no sample was ever written to its track. SimpleMp4FrameMuxer.release() only guarded against the never-started case, so a started-but-empty muxer made release() throw. Because release() runs from SimpleVideoEncoder's finally block, that throw propagates out to createVideoOf, which treats release() as safe cleanup; the encoder is left dangling and the orphan video file is never deleted. Only call stop() when at least one sample was written; muxer.release() on a started-but-not-stopped muxer is safe. --- .../io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt index e32af9bb44b..81e5015a0fb 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt @@ -67,9 +67,9 @@ internal class SimpleMp4FrameMuxer(path: String, fps: Float) : SimpleFrameMuxer } override fun release() { - // stop() throws if the muxer was never started (e.g. no frame was ever muxed), so we guard it - // to ensure release() is always reached and the underlying resources are freed - if (started) { + // stop() throws unless the muxer was started AND at least one sample was written, so we guard it + // to ensure muxer.release() is always reached and the underlying resources are freed + if (started && videoFrames > 0) { muxer.stop() } muxer.release() From ff29522cc722184f589bbbf3c13af4761f1091fb Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 23 Jul 2026 10:34:18 +0000 Subject: [PATCH 4/5] Format code --- .../java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt index 81e5015a0fb..0063cf636e4 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt @@ -67,7 +67,8 @@ internal class SimpleMp4FrameMuxer(path: String, fps: Float) : SimpleFrameMuxer } override fun release() { - // stop() throws unless the muxer was started AND at least one sample was written, so we guard it + // stop() throws unless the muxer was started AND at least one sample was written, so we guard + // it // to ensure muxer.release() is always reached and the underlying resources are freed if (started && videoFrames > 0) { muxer.stop() From 3066764fe57a2298d54977c731d7ce8671001754 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 23 Jul 2026 12:50:34 +0200 Subject: [PATCH 5/5] fix(replay): Guard each native release so cleanup never propagates The finally block in SimpleVideoEncoder.release() released the codec, surface, and muxer without guards. release() is treated by callers such as createVideoOf as safe cleanup, but MediaMuxer.stop() (reached via frameMuxer.release()) can still throw IllegalStateException on a genuine file-finalization failure even when samples were written. That would skip the remaining releases and propagate out of release(). Guard each release independently so failing to free one resource neither skips the others nor escapes to the caller. --- .../replay/video/SimpleVideoEncoder.kt | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 8490bd80150..d0cafd9f1a9 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -291,10 +291,21 @@ internal class SimpleVideoEncoder( options.logger.log(DEBUG, "Failed to properly release video encoder", e) } finally { // always release the native resources, even if draining/stopping the codec above threw (e.g. - // when the encoder failed to fully start), otherwise they leak (CloseGuard warning) - mediaCodec.release() - surface?.release() - frameMuxer.release() + // when the encoder failed to fully start), otherwise they leak (CloseGuard warning). guard + // each + // call so failing to release one resource neither skips the others nor propagates to callers, + // which treat release() as safe cleanup + releaseQuietly("MediaCodec") { mediaCodec.release() } + releaseQuietly("Surface") { surface?.release() } + releaseQuietly("MediaMuxer") { frameMuxer.release() } + } + } + + private inline fun releaseQuietly(name: String, block: () -> Unit) { + try { + block() + } catch (e: RuntimeException) { + options.logger.log(DEBUG, "Failed to release $name", e) } } }