diff --git a/kotlin/src/androidTest/kotlin/app/rive/runtime/kotlin/core/RiveArtboardRendererTest.kt b/kotlin/src/androidTest/kotlin/app/rive/runtime/kotlin/core/RiveArtboardRendererTest.kt index 55231371..7431a48f 100644 --- a/kotlin/src/androidTest/kotlin/app/rive/runtime/kotlin/core/RiveArtboardRendererTest.kt +++ b/kotlin/src/androidTest/kotlin/app/rive/runtime/kotlin/core/RiveArtboardRendererTest.kt @@ -3,11 +3,14 @@ package app.rive.runtime.kotlin.core import android.graphics.SurfaceTexture import android.view.Surface import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import app.rive.runtime.kotlin.test.R 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.Before import org.junit.Test import org.junit.runner.RunWith import java.util.concurrent.CountDownLatch @@ -17,7 +20,17 @@ import java.util.concurrent.locks.ReentrantLock @RunWith(AndroidJUnit4::class) class RiveArtboardRendererTest { - private val testUtils = TestUtils() + + companion object { + private const val TIMEOUT_MS = 1000L + private const val POLL_INTERVAL_MS = 10L + } + + @Before + fun setup() { + // Ensure native libraries are loaded for JNI calls. + TestUtils().context + } @Test fun createRenderer() { @@ -56,345 +69,433 @@ class RiveArtboardRendererTest { } /** - * Tests for a race where the renderer is deleted while a frame is being drawn, using a - * controller subclass that blocks the getter for activeArtboard. + * Verifies delete() cannot run concurrently with draw(). + * + * Blocks inside [Artboard.draw] under [Renderer.frameLock], starts [RiveArtboardRenderer.delete] + * on another thread, and asserts delete blocks until draw releases the lock. + * + * Intentionally uses [RiveFileController] **without** a [File]: this test targets + * [Renderer.frameLock] only, not [File.fileLock]. */ @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 - val duringActiveArtboardLatch = CountDownLatch(1) - // Latch to block the activeArtboard getter until we have deleted the renderer - val afterDeleteLatch = CountDownLatch(1) + // Latch: draw has entered the blocking artboard override (draw() holds frameLock). + val insideDrawLatch = CountDownLatch(1) + // Latch: test releases this to let draw() finish and exit frameLock. + val releaseDrawLatch = CountDownLatch(1) - // The renderer needs a valid artboard to proceed to accessing the cppPointer + // The renderer needs a valid artboard to proceed with accessing the cppPointer val dummyArtboard = object : Artboard(unsafeCppPointer = 1L, fileLock = ReentrantLock()) { - // Override draw to do nothing, avoiding the thread affinity checks in the real draw(). + // Override draw to add blocking and signal we're inside the synchronized block; + // this avoids the thread affinity checks in the real draw() override fun draw( rendererAddress: Long, fit: Fit, alignment: Alignment, scaleFactor: Float ) { + // Signal that draw reached Artboard.draw (frameLock still held by draw()). + insideDrawLatch.countDown() + // Hold frameLock until the test releases releaseDrawLatch. + releaseDrawLatch.awaitOrFail("Timed out waiting for test to release draw latch") } - } - val latchedController = object : RiveFileController() { - // Called by the renderer's `draw()` method *before* accessing the cppPointer but *after* - // checking `hasCppObject` and `isActive`. - override var activeArtboard: Artboard? - get() { - // Signal that we are in the getter - duringActiveArtboardLatch.countDown() - // Wait until we have deleted the renderer - afterDeleteLatch.await(timeout, TimeUnit.MILLISECONDS) - return dummyArtboard - } - set(value) { - super.activeArtboard = value - } + // Prevent any delete on fake pointer (1L). + override fun cppDelete(pointer: Long) {} } - latchedController.isActive = true - latchedController.activeArtboard = dummyArtboard - val renderer = RiveArtboardRenderer(controller = latchedController) + val controller = RiveFileController(activeArtboard = dummyArtboard) + controller.isActive = true + val renderer = RiveArtboardRenderer(controller = controller) renderer.make() - // Start draw() in a background thread, simulating the worker thread. - // It will block in activeArtboard getter. + // Capture any exception thrown in the background threads + val exceptionRef = AtomicReference() + + // Start draw() in a background thread - this will hold frameLock for the duration of draw() val drawThread = Thread { - renderer.draw() + try { + renderer.draw() + } catch (e: Throwable) { + exceptionRef.set(e) + } } drawThread.start() - // Wait for the getter to be entered - duringActiveArtboardLatch.await(timeout, TimeUnit.MILLISECONDS) - - // Simulate deletion during draw(), nulling the cppPointer. - renderer.delete() - // Let the getter return and draw() proceed - afterDeleteLatch.countDown() - - drawThread.join(timeout) - } - - /** - * Tests for a race where an artboard is disposed while being drawn, specifically after it has - * entered the synchronized block in draw() but before dereferencing the cppPointer. - */ - @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. - val readyForRelease = CountDownLatch(1) - // Signals that the main thread has released the artboard, and draw() can continue. - val afterRelease = CountDownLatch(1) + insideDrawLatch.awaitOrFail("draw() did not reach Artboard.draw in time") - // A lock we can reference, unlike the default private Artboard file lock. - val artboardLock = ReentrantLock() - - // An artboard that synchronizes with the main thread to simulate being released while - // being drawn. - val latchingArtboard = object : Artboard(unsafeCppPointer = 1L, fileLock = artboardLock) { - override fun draw( - rendererAddress: Long, - fit: Fit, - alignment: Alignment, - scaleFactor: Float - ) { - synchronized(artboardLock) { - readyForRelease.countDown() - // Wait for the test to dispose the artboard. - // We expect this to timeout as the artboard's locking prevents the main thread - // from disposing, so afterRelease cannot be counted down. - afterRelease.await(timeout, TimeUnit.MILLISECONDS) - // Simulate the critical part of draw(): dereference the artboard pointer. - cppPointer - } + // Counted down at the start of delete() so waitUntil knows the thread entered delete + // (and is blocked on frameLock, not still starting up). + val deleteInvoked = CountDownLatch(1) + // Start delete() in a separate thread - it should block trying to acquire frameLock + val deleteThread = Thread { + try { + deleteInvoked.countDown() + renderer.delete() + } catch (e: Throwable) { + exceptionRef.set(e) } - - // Need to override to avoid calling the real native delete on address 1L - override fun cppDelete(pointer: Long) {} } + deleteThread.start() - val controller = RiveFileController(activeArtboard = latchingArtboard) - controller.isActive = true + waitUntil( + message = "Expected delete to be blocked waiting for frameLock while draw is executing" + ) { + deleteInvoked.count == 0L && deleteThread.state == Thread.State.BLOCKED + } - val renderer = RiveArtboardRenderer(controller = controller) - renderer.make() + // Now allow draw() to complete and release the lock + releaseDrawLatch.countDown() - // Simulate the worker thread calling draw() - val drawThread = Thread { - // Calls the controller's active artboard to draw - renderer.draw() - } - drawThread.start() + // Wait for draw thread to finish (releases frameLock) + drawThread.join(TIMEOUT_MS) - // Wait until renderer.draw() has entered Artboard.draw(). - readyForRelease.await(timeout, TimeUnit.MILLISECONDS) + // Now delete should be able to complete + deleteThread.join(TIMEOUT_MS) - // Dispose the artboard by releasing the one place that references it. - controller.activeArtboard = null + // Verify delete completed successfully (thread is no longer alive) assertFalse( - "Expected latchingArtboard to have been disposed", - latchingArtboard.hasCppObject + "Expected delete to complete after frameLock was released", + deleteThread.isAlive ) - // Let draw() continue - afterRelease.countDown() - - drawThread.join(timeout) + // Verify no exception was thrown + val exception = exceptionRef.get() + assert(exception == null) { + "Expected no exception with proper locking. " + + "Got: ${exception?.javaClass?.simpleName}: ${exception?.message}" + } } /** - * Tests for a race where an artboard is disposed while being drawn, specifically before it has - * entered the synchronized block in draw(). + * Verifies draw() holds [File.lock] so UI-thread artboard mutations block until the frame + * completes. + * + * Uses [loadEmptyFile] so [RiveFileController.file] is set — see that helper for why a real + * [File] is required (without it, draw and [RiveFileController.activeArtboard] synchronize on + * different fallback objects and file-lock regressions do not fail). */ @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) - // Signals that the main thread has released the artboard, and draw() can continue. - val afterRelease = CountDownLatch(1) + fun drawHoldsFileLockBlocksActiveArtboardMutation() { + assertActiveArtboardMutationBlocksOnFileLockDuringDraw() + } - // A lock we can reference, unlike the default private Artboard file lock. - val artboardLock = ReentrantLock() + /** + * While draw() is inside [Artboard.draw], [RiveFileController.activeArtboard] mutations must + * block on [File.fileLock] until the frame completes. + * + * Backed by [loadEmptyFile] so draw and the setter share one lock — see [loadEmptyFile]. + */ + @Test + fun disposeArtboardDuringFrameAfterEnteringSyncBlock() { + assertActiveArtboardMutationBlocksOnFileLockDuringDraw(assertArtboardDisposal = true) + } - // An artboard that synchronizes with the main thread to simulate being released right - // before being drawn. - val latchingArtboard = object : Artboard(unsafeCppPointer = 1L, fileLock = artboardLock) { - override fun draw( - rendererAddress: Long, - fit: Fit, - alignment: Alignment, - scaleFactor: Float - ) { - readyForRelease.countDown() - afterRelease.await() - super.draw(rendererAddress, fit, alignment, scaleFactor) - } + /** + * Same [File.fileLock] requirement as [disposeArtboardDuringFrameAfterEnteringSyncBlock], + * with the latching override resuming into the real [Artboard.draw] after the latch. + */ + @Test + fun disposeArtboardDuringFrameBeforeEnteringSyncBlock() { + assertActiveArtboardMutationBlocksOnFileLockDuringDraw( + callSuperDrawAfterLatch = true, + assertArtboardDisposal = true, + ) + } - // Do nothing, avoiding the thread affinity checks in the real draw(). - override fun cppDrawAligned( - cppPointer: Long, rendererPointer: Long, - fit: Fit, alignment: Alignment, - scaleFactor: Float - ) { - } + /** + * Regression test for #424: resize reads renderer width/height while draw() holds frameLock. + * + * Pauses inside [RiveFileController.fit] (read during resize) so another thread can call + * [RiveArtboardRenderer.delete]. Delete blocks on frameLock; resize must not touch a disposed + * renderer pointer when the pause ends. + */ + @Test + fun deleteRendererBetweenCppObjectCheckAndDimensionRead_doesNotCrash() { + val readyForDelete = CountDownLatch(1) + val resumeAfterDelete = CountDownLatch(1) + val exceptionRef = AtomicReference(null) - // Need to override to avoid calling the real native delete on address 1L - override fun cppDelete(pointer: Long) {} + val controller = object : RiveFileController() { + override var fit: Fit + get() { + readyForDelete.countDown() + // Deterministic pause during resizeArtboard() (fit is read for layout sizing). + resumeAfterDelete.awaitOrFail("Timed out waiting to resume draw after delete") + return super.fit + } + set(value) { + super.fit = value + } } - - val controller = RiveFileController(activeArtboard = latchingArtboard) + controller.fit = Fit.LAYOUT // Sets requireArtboardResize = true controller.isActive = true val renderer = RiveArtboardRenderer(controller = controller) renderer.make() - // Simulate the worker thread calling draw() val drawThread = Thread { - // Calls the controller's active artboard to draw - renderer.draw() + try { + renderer.draw() + } catch (e: Throwable) { + exceptionRef.set(e) + } } drawThread.start() - // Wait until renderer.draw() has entered Artboard.draw(). - readyForRelease.await(timeout, TimeUnit.MILLISECONDS) - - // Dispose the artboard by releasing the one place that references it. - controller.activeArtboard = null - assertFalse( - "Expected latchingArtboard to have been disposed", - latchingArtboard.hasCppObject + readyForDelete.awaitOrFail( + "draw() did not reach the fit getter in time; race was not induced." ) - // Let draw() continue - afterRelease.countDown() + // delete() blocks on frameLock while draw is paused in the fit getter (still inside draw()). + // Start delete on a background thread, then countDown resume so resize can finish while + // delete waits — same pattern as deleteRendererDuringFrame. + val deleteThread = Thread { + try { + renderer.delete() + } catch (e: Throwable) { + exceptionRef.set(e) + } + } + deleteThread.start() + + resumeAfterDelete.countDown() + + drawThread.join(TIMEOUT_MS) + deleteThread.join(TIMEOUT_MS) - drawThread.join(timeout) + val exception = exceptionRef.get() + assertTrue( + "Expected no exception when deleting renderer during resize race, " + + "but got: ${exception?.javaClass?.simpleName}: ${exception?.message}", + exception == null + ) } /** - * 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. + * Loads [R.raw.empty] for tests that assert [File.fileLock] behaviour. + * + * [RiveArtboardRenderer.draw] and [RiveFileController.activeArtboard] both use + * `synchronized(file?.fileLock ?: this)`. When [RiveFileController.file] is null those fallbacks + * are **different** objects (the renderer instance vs the controller instance), so the old + * dispose tests could pass even with `fileLock` removed from `draw()`. Wiring a real [File] + * forces both code paths onto [File.fileLock], matching production [RiveAnimationView] usage. */ - @Test - fun deleteRendererDuringResizeArtboard() { - // Ensure native libraries are loaded for JNI calls. - testUtils.context + private fun loadEmptyFile(): File { + val bytes = InstrumentationRegistry.getInstrumentation().targetContext.resources + .openRawResource(R.raw.empty) + .readBytes() + return File(bytes) + } - val timeout = 1000L - // Latch to signal we've entered resizeArtboard() - val duringResizeLatch = CountDownLatch(1) - // Latch to block until we have deleted the renderer - val afterDeleteLatch = CountDownLatch(1) + /** + * Controller, renderer, and latching artboard for [File.fileLock] tests — see [loadEmptyFile]. + * + * [RiveArtboardRenderer.draw] blocks in [Artboard.draw] until [releaseDrawLatch] is counted + * down, while holding the same [File.lock] used by [RiveFileController.activeArtboard] + * mutations. + */ + private data class FileLockDrawFixture( + val latchingArtboard: Artboard, + val controller: RiveFileController, + val renderer: RiveArtboardRenderer, + ) - val controller = RiveFileController() - controller.fit = Fit.LAYOUT // This sets requireArtboardResize to true - controller.isActive = true + /** + * Builds a [FileLockDrawFixture]: real [File] from [R.raw.empty], artboard whose [Artboard.draw] + * pauses on [releaseDrawLatch], and an active controller/renderer pair. + * + * @param timeout Passed to [awaitOrFail] while [Artboard.draw] is blocked; defaults to + * [TIMEOUT_MS]. + * @param insideDrawLatch Counted down when [Artboard.draw] is entered. + * @param releaseDrawLatch Unblocks [Artboard.draw] when the test is ready to finish the frame. + * @param callSuperDrawAfterLatch When true, resumes into [Artboard.draw] after the latch and + * stubs [Artboard.cppDrawAligned] to avoid native draw / thread-affinity checks. + */ + private fun fileLockDrawFixture( + insideDrawLatch: CountDownLatch, + releaseDrawLatch: CountDownLatch, + timeout: Long = TIMEOUT_MS, + callSuperDrawAfterLatch: Boolean = false, + ): FileLockDrawFixture { + val file = loadEmptyFile() + val latchingArtboard = if (callSuperDrawAfterLatch) { + object : Artboard(unsafeCppPointer = 1L, fileLock = file.lock) { + override fun draw( + rendererAddress: Long, + fit: Fit, + alignment: Alignment, + scaleFactor: Float + ) { + insideDrawLatch.countDown() + releaseDrawLatch.awaitOrFail( + "Timed out waiting for test to release draw latch", + timeoutMs = timeout, + ) + super.draw(rendererAddress, fit, alignment, scaleFactor) + } - // Create a custom renderer that overrides resizeArtboard() to add blocking, - // simulating the race condition where the renderer is deleted during resize - val latchingRenderer = object : RiveArtboardRenderer(controller = controller) { - override fun resizeArtboard() { - // Signal we've entered resizeArtboard() - duringResizeLatch.countDown() + // Do nothing, avoiding the thread affinity checks in the real draw(). + override fun cppDrawAligned( + cppPointer: Long, rendererPointer: Long, + fit: Fit, alignment: Alignment, + scaleFactor: Float + ) { + } - // Block until the renderer is deleted - this simulates the race where - // the renderer could be deleted while resizeArtboard() is executing - afterDeleteLatch.await(timeout, TimeUnit.MILLISECONDS) + override fun cppDelete(pointer: Long) {} + } + } else { + object : Artboard(unsafeCppPointer = 1L, fileLock = file.lock) { + override fun draw( + rendererAddress: Long, + fit: Fit, + alignment: Alignment, + scaleFactor: Float + ) { + insideDrawLatch.countDown() + releaseDrawLatch.awaitOrFail( + "Timed out waiting for test to release draw latch", + timeoutMs = timeout, + ) + } - // Call the parent's resizeArtboard() which will check hasCppObject - // (the fix) and return early if disposed, preventing a crash - super.resizeArtboard() + override fun cppDelete(pointer: Long) {} } } - latchingRenderer.make() + val controller = RiveFileController(file = file, activeArtboard = latchingArtboard) + controller.isActive = true + val renderer = RiveArtboardRenderer(controller = controller) + renderer.make() + return FileLockDrawFixture(latchingArtboard, controller, renderer) + } - // Capture any exception thrown in the background thread + /** + * Runs draw() on a background thread while blocked in [Artboard.draw], starts an + * [RiveFileController.activeArtboard] = null mutation, and asserts the mutation blocks on + * [File.fileLock] until draw finishes. + * + * @param callSuperDrawAfterLatch Passed to [fileLockDrawFixture]; when true, the latching + * artboard resumes into [Artboard.draw] after the latch. + * @param assertArtboardDisposal When true, asserts the latching artboard is not disposed while + * blocked and is disposed after the frame completes. + */ + private fun assertActiveArtboardMutationBlocksOnFileLockDuringDraw( + callSuperDrawAfterLatch: Boolean = false, + assertArtboardDisposal: Boolean = false, + ) { + val insideDrawLatch = CountDownLatch(1) + val releaseDrawLatch = CountDownLatch(1) + val mutationComplete = CountDownLatch(1) val exceptionRef = AtomicReference() - // Start draw() in a background thread + val fixture = fileLockDrawFixture( + insideDrawLatch = insideDrawLatch, + releaseDrawLatch = releaseDrawLatch, + callSuperDrawAfterLatch = callSuperDrawAfterLatch, + ) + val drawThread = Thread { try { - latchingRenderer.draw() + fixture.renderer.draw() } catch (e: Throwable) { exceptionRef.set(e) } } drawThread.start() - // Wait for resizeArtboard() to be entered - duringResizeLatch.await(timeout, TimeUnit.MILLISECONDS) + insideDrawLatch.awaitOrFail("draw() did not reach Artboard.draw in time") + + val mutationThread = Thread { + try { + fixture.controller.activeArtboard = null + mutationComplete.countDown() + } catch (e: Throwable) { + exceptionRef.set(e) + } + } + mutationThread.start() + + waitUntil( + message = "Expected activeArtboard mutation to block while draw holds fileLock" + ) { + mutationThread.state == Thread.State.BLOCKED + } + + if (assertArtboardDisposal) { + assertTrue( + "Artboard should not be disposed while draw holds fileLock", + fixture.latchingArtboard.hasCppObject + ) + } - // Delete the renderer while resizeArtboard() is blocked - latchingRenderer.delete() + releaseDrawLatch.countDown() - // Let resizeArtboard() continue - the hasCppObject check should prevent a crash - afterDeleteLatch.countDown() + drawThread.join(TIMEOUT_MS) + mutationThread.join(TIMEOUT_MS) - drawThread.join(timeout) + mutationComplete.awaitOrFail( + "activeArtboard mutation should complete after draw finishes" + ) + + if (assertArtboardDisposal) { + assertFalse( + "Expected latchingArtboard to have been disposed after draw completed", + fixture.latchingArtboard.hasCppObject + ) + } - // Verify no exception was thrown - the fix should prevent the crash val exception = exceptionRef.get() assert(exception == null) { - "Expected no exception when renderer is deleted during resizeArtboard(). " + + "Expected no exception with proper fileLock. " + "Got: ${exception?.javaClass?.simpleName}: ${exception?.message}" } + + fixture.controller.isActive = false + fixture.controller.release() } /** - * Regression test for a race where resizeArtboard() reads hasCppObject, then the renderer is - * deleted before width/height are read. + * Polls until [condition] is true or [timeoutMs] elapses. + * + * Prefer this over fixed [Thread.sleep] when waiting for concurrent threads to reach an + * expected state: fast on healthy runners, tolerant of slow CI emulators. Lock tests should + * pass a [condition] that detects blocking (e.g. [Thread.State.BLOCKED]), not merely + * [Thread.isAlive]. * - * The fit getter is used as a deterministic pause point between those operations. + * @param timeoutMs Maximum time to poll before throwing [AssertionError] with [message]. + * @param message Failure message when [condition] never becomes true. + * @param condition Expected concurrent state; polled every [POLL_INTERVAL_MS]. */ - @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(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) + private fun waitUntil( + timeoutMs: Long = TIMEOUT_MS, + message: String, + condition: () -> Boolean, + ) { + val deadline = System.currentTimeMillis() + timeoutMs + while (!condition()) { + if (System.currentTimeMillis() >= deadline) { + throw AssertionError(message) } + Thread.sleep(POLL_INTERVAL_MS) } - 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 - ) + /** + * Awaits this latch within [timeoutMs], failing the test with [message] if it times out. + * + * @param message Failure message when the latch does not reach zero in time. + * @param timeoutMs Maximum time to wait; defaults to [TIMEOUT_MS]. + * @param timeUnit Unit for [timeoutMs]; defaults to milliseconds. + */ + private fun CountDownLatch.awaitOrFail( + message: String, + timeoutMs: Long = TIMEOUT_MS, + timeUnit: TimeUnit = TimeUnit.MILLISECONDS, + ) { + if (!await(timeoutMs, timeUnit)) { + throw AssertionError(message) + } } } diff --git a/kotlin/src/main/java/app/rive/runtime/kotlin/controllers/RiveFileController.kt b/kotlin/src/main/java/app/rive/runtime/kotlin/controllers/RiveFileController.kt index 3fbfe966..25a6adff 100644 --- a/kotlin/src/main/java/app/rive/runtime/kotlin/controllers/RiveFileController.kt +++ b/kotlin/src/main/java/app/rive/runtime/kotlin/controllers/RiveFileController.kt @@ -100,7 +100,9 @@ class RiveFileController internal constructor( /** * Whether this controller is active or not. If this is false, it will prevent advancing or * drawing. + * Marked as `@Volatile` to ensure immediate visibility across worker & UI threads */ + @Volatile var isActive = false /** diff --git a/kotlin/src/main/java/app/rive/runtime/kotlin/renderers/RiveArtboardRenderer.kt b/kotlin/src/main/java/app/rive/runtime/kotlin/renderers/RiveArtboardRenderer.kt index 957cf0d8..756f1ef1 100644 --- a/kotlin/src/main/java/app/rive/runtime/kotlin/renderers/RiveArtboardRenderer.kt +++ b/kotlin/src/main/java/app/rive/runtime/kotlin/renderers/RiveArtboardRenderer.kt @@ -1,6 +1,5 @@ package app.rive.runtime.kotlin.renderers -import androidx.annotation.VisibleForTesting import androidx.annotation.WorkerThread import app.rive.RiveLog import app.rive.core.traceSection @@ -37,32 +36,26 @@ open class RiveArtboardRenderer( } } + /** + * Resizes the active artboard to match the renderer surface. + * + * Must be called with [frameLock] held, then `controller.file?.fileLock` (in that order). + * [draw] is the only call site; locks are not taken here so ordering stays centralized. + */ @WorkerThread - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - open fun resizeArtboard() { + private fun resizeArtboard() { if (fit == Fit.LAYOUT) { traceSection("Rive/Layout/ResizeArtboard") { - // 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?.fileLock ?: this) { - controller.activeArtboard?.apply { - width = newWidth - height = newHeight - } + val newWidth = width / scaleFactor + val newHeight = height / scaleFactor + controller.activeArtboard?.apply { + width = newWidth + height = newHeight } } } else { traceSection("Rive/Layout/ResetArtboardSize") { - synchronized(controller.file?.fileLock ?: this) { - controller.activeArtboard?.resetArtboardSize() - } + controller.activeArtboard?.resetArtboardSize() } } } @@ -70,16 +63,27 @@ open class RiveArtboardRenderer( // Be aware of thread safety! @WorkerThread override fun draw() { - if (controller.requireArtboardResize.getAndSet(false)) { - resizeArtboard() - } - - // Deref and draw under frameLock + // Resize and draw under frameLock synchronized(frameLock) { // Early out for deleted renderer or inactive controller. + // hasCppObject is only mutated under frameLock; isActive may change on other threads. if (!hasCppObject || !controller.isActive) return - controller.activeArtboard?.draw(cppPointer, fit, alignment, scaleFactor = scaleFactor) + // Protect both resize and draw with fileLock (frameLock first, always). Matches + // controller.advance() and prevents UI-thread file/artboard mutations mid-frame. + synchronized(controller.file?.fileLock ?: this) { + // Re-check isActive only; hasCppObject remains stable while frameLock is held. + if (!controller.isActive) return + if (controller.requireArtboardResize.getAndSet(false)) { + resizeArtboard() + } + controller.activeArtboard?.draw( + cppPointer, + fit, + alignment, + scaleFactor = scaleFactor + ) + } } } @@ -110,9 +114,4 @@ open class RiveArtboardRenderer( controller.selectArtboard() start() } - - override fun disposeDependencies() { - // Lock to make sure things are disposed in an orderly manner. - synchronized(controller.file?.fileLock ?: this) { super.disposeDependencies() } - } }