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

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