fix(Android): Race condition between resize and renderer deletion (#11831) 086750cc4a

There is a time-of-check, time-of-use race (TOCTOU) between the renderer being deleted and checking the dimensions. We did a check to early return but it wasn't under frame lock, so the result could change (by deletion) between the check and the retrieval. This splits `resizeArtboard` to have two lock sections (in the case of fit = Layout): one to get the dimensions under frame lock, and another to mutate the artboard under file lock.

Co-authored-by: Erik <erik@rive.app>

Author: ErikUggeldahl

.rive_head

@@ -1 +1 @@
-d9776b5c2e2e1e9bae20c9ccb44858ed3292ea8f
+086750cc4a91e37ae3760124805eb9369d3ceff5

kotlin/src/androidTest/kotlin/app/rive/runtime/kotlin/core/RiveArtboardRendererTest.kt

@@ -7,6 +7,7 @@ import app.rive.runtime.kotlin.SharedSurface
 import app.rive.runtime.kotlin.controllers.RiveFileController
 import app.rive.runtime.kotlin.renderers.RiveArtboardRenderer
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 import java.util.concurrent.CountDownLatch
@@ -60,6 +61,9 @@ class RiveArtboardRendererTest {
      */
     @Test
     fun deleteRendererDuringFrame() {
+        // Ensure native libraries are loaded for JNI calls.
+        testUtils.context
+
         // Keep this low, as the test times out during non-critical activeArtboard access.
         val timeout = 100L
         // Latch to block signal we are passed the `hasCppObject` check in the draw() method
@@ -124,6 +128,9 @@ class RiveArtboardRendererTest {
      */
     @Test
     fun disposeArtboardDuringFrameAfterEnteringSyncBlock() {
+        // Ensure native libraries are loaded for JNI calls.
+        testUtils.context
+
         // Keep this low, as we expect it to timeout.
         val timeout = 1000L
         // Signals that the artboard has entered draw() and is about to dereference the cppPointer.
@@ -193,6 +200,9 @@ class RiveArtboardRendererTest {
      */
     @Test
     fun disposeArtboardDuringFrameBeforeEnteringSyncBlock() {
+        // Ensure native libraries are loaded for JNI calls.
+        testUtils.context
+
         val timeout = 1000L
         // Signals that the artboard has entered draw() and is about to dereference the cppPointer.
         val readyForRelease = CountDownLatch(1)
@@ -258,12 +268,16 @@ class RiveArtboardRendererTest {
     }
 
     /**
-     * Tests that the renderer can be safely deleted while resizeArtboard() is executing.
-     * The fix adds a hasCppObject check at the start of resizeArtboard() to prevent
-     * accessing disposed C++ objects when accessing width/height properties.
+     * Tests that the renderer can be safely deleted while resizeArtboard() is executing. The fix
+     * reads renderer liveness and surface dimensions together under frameLock (in the Fit.LAYOUT
+     * case where this is required), then applies artboard mutations under the file lock to avoid
+     * disposal races.
      */
     @Test
     fun deleteRendererDuringResizeArtboard() {
+        // Ensure native libraries are loaded for JNI calls.
+        testUtils.context
+
         val timeout = 1000L
         // Latch to signal we've entered resizeArtboard()
         val duringResizeLatch = CountDownLatch(1)
@@ -324,4 +338,63 @@ class RiveArtboardRendererTest {
                     "Got: ${exception?.javaClass?.simpleName}: ${exception?.message}"
         }
     }
+
+    /**
+     * Regression test for a race where resizeArtboard() reads hasCppObject, then the renderer is
+     * deleted before width/height are read.
+     *
+     * The fit getter is used as a deterministic pause point between those operations.
+     */
+    @Test
+    fun deleteRendererBetweenCppObjectCheckAndDimensionRead_doesNotCrash() {
+        // Ensure native libraries are loaded for JNI calls.
+        testUtils.context
+
+        val timeoutMs = 1000L
+        val readyForDelete = CountDownLatch(1)
+        val resumeAfterDelete = CountDownLatch(1)
+        val exceptionRef = AtomicReference<Throwable?>(null)
+
+        val controller = object : RiveFileController() {
+            override var fit: Fit
+                get() {
+                    readyForDelete.countDown()
+                    resumeAfterDelete.await(timeoutMs, TimeUnit.MILLISECONDS)
+                    return super.fit
+                }
+                set(value) {
+                    super.fit = value
+                }
+        }
+        controller.fit = Fit.LAYOUT // Sets requireArtboardResize = true
+
+        val renderer = RiveArtboardRenderer(controller = controller)
+        renderer.make()
+
+        val drawThread = Thread {
+            try {
+                renderer.draw()
+            } catch (e: Throwable) {
+                exceptionRef.set(e)
+            }
+        }
+        drawThread.start()
+
+        assertTrue(
+            "draw() did not reach the fit getter in time; race was not induced.",
+            readyForDelete.await(timeoutMs, TimeUnit.MILLISECONDS)
+        )
+
+        renderer.delete()
+        resumeAfterDelete.countDown()
+
+        drawThread.join(timeoutMs)
+
+        val exception = exceptionRef.get()
+        assertTrue(
+            "Expected no exception when deleting renderer during resize race, " +
+                    "but got: ${exception?.javaClass?.simpleName}: ${exception?.message}",
+            exception == null
+        )
+    }
 }

kotlin/src/main/java/app/rive/runtime/kotlin/renderers/RiveArtboardRenderer.kt

@@ -39,25 +39,34 @@ open class RiveArtboardRenderer(
     @WorkerThread
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     open fun resizeArtboard() {
-        if (!hasCppObject) return
-
         if (fit == Fit.LAYOUT) {
-            val newWidth = width / scaleFactor
-            val newHeight = height / scaleFactor
-            controller.activeArtboard?.apply {
-                width = newWidth
-                height = newHeight
+            // Read surface dimensions under frameLock so delete() cannot null cppPointer between
+            // hasCppObject checks and width/height dereference.
+            val (newWidth, newHeight) = synchronized(frameLock) {
+                if (!hasCppObject || !controller.isActive) return
+                Pair(width / scaleFactor, height / scaleFactor)
+            }
+
+            // Acquire file lock only after the frameLock section to avoid lock-order inversion and
+            // serialize artboard mutations with controller/file lifecycle operations.
+            synchronized(controller.file?.lock ?: this) {
+                controller.activeArtboard?.apply {
+                    width = newWidth
+                    height = newHeight
+                }
             }
         } else {
-            controller.activeArtboard?.resetArtboardSize()
+            synchronized(controller.file?.lock ?: this) {
+                controller.activeArtboard?.resetArtboardSize()
+            }
         }
     }
 
     // Be aware of thread safety!
     @WorkerThread
     override fun draw() {
         if (controller.requireArtboardResize.getAndSet(false)) {
-            synchronized(controller.file?.lock ?: this) { resizeArtboard() }
+            resizeArtboard()
         }
 
         // Deref and draw under frameLock