fix(replay): Release MediaMuxer when the encoder fails to start (#5607)

* 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 <noreply@anthropic.com>

* changelog

* 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.

* Format code

* 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.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>

Author: 3 people

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
 

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()

sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleMp4FrameMuxer.kt

@@ -67,9 +67,10 @@ 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()

sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt

@@ -287,12 +287,25 @@ internal class SimpleVideoEncoder(
       onClose?.invoke()
       drainCodec(true)
       mediaCodec.stop()
-      mediaCodec.release()
-      surface?.release()
-
-      frameMuxer.release()
-    } catch (e: Throwable) {
+    } 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). 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)
     }
   }
 }

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<IllegalStateException> {
+      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

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()
   }