[#424] Fix: app.rive.runtime.kotlin.core.errors.RiveException: Accessing disposed C++ object RiveArtboardRenderer.#429
Conversation
…: Accessing disposed C++ object RiveArtboardRenderer. [rive-app#424] Fix: app.rive.runtime.kotlin.core.errors.RiveException: Accessing disposed C++ object RiveArtboardRenderer. - The crash is caused by calling resizeArtboard which attempts to get the width from the disposed C++ object, so to protect against this we move the call to inside of the frame lock. - This nesting of the file lock inside of the frame lock looks to be correctly supported by the disposeDependencies function first releasing the file lock and then releasing the frame lock.
|
I haven't been able to repro the crash locally yet, we have only seen it in the field at a very low % (~0.025%) but will try the method the other user has described to confirm the fix works. |
|
Here is my attempt at reproducing the crash, but it doesn't seem to repro: https://github.com/dabrosch/rive-android/tree/dabrosch/10.4.4-with-bug-demo |
* fix(android): crash on artboard resizing * test: add test to verify the crash Co-authored-by: Gordon <pggordonhayes@gmail.com>
- Switched both Draw and resizeArtBoard to be under both the file lock and the frame lock. - Removed test for resizeArtboard and made it private because we are already testing that the CppObject cannot be changed during draw. - Updated some comments, I am still wondering if we really need the file lock before disposingDependencies, because the objects it is releasing look like they could be mutated even when the file or even frame lock is held. - Tests were not even able to be ran without the Rive.init, so I added that.
| val afterDeleteLatch = CountDownLatch(1) | ||
|
|
||
| // The renderer needs a valid artboard to proceed to accessing the cppPointer | ||
| val timeout = 1000L |
There was a problem hiding this comment.
We can try with lower numbers for the timeout, I just wanted to be safe.
| open fun resizeArtboard() { | ||
| if (!hasCppObject) return |
There was a problem hiding this comment.
This was not protected from race conditions because it didn't have the frameLock.
|
Thanks for the contribution @dabrosch. I'll tag @ErikUggeldahl to review this |
| synchronized(controller.file?.lock ?: this) { | ||
| super.disposeDependencies() | ||
| } |
There was a problem hiding this comment.
I just realized this could cause a deadlock with the above code. I am not even fully convinced that we really do need a File Lock on this, but if we do keep it, the order of lock obtainment needs to match the order in draw, so Frame Lock then File Lock.
… not also be synchronized on the file lock. If left as is we would get a deadlock with the above draw function which gets the framelock and then gets the file lock.
It would be nice to stop having this crash :-) I just merged it with main. |
|
bump |
…lock ordering fix. Keep resize and draw under frameLock then fileLock, make resizeArtboard private with no internal locking, and drop disposeDependencies fileLock override to avoid lock-order inversion with draw().
…ive volatile - The prior dispose-during-draw tests used RiveFileController without a File, so draw() and activeArtboard synchronized on different fallback locks and would pass even when fileLock was removed from draw(). - Rewrote the suite against a real empty .riv file with shared fixtures, added a rive-app#424 resize/delete regression test, and deduped the file-lock mutation scenarios. - Marked controller.isActive @volatile so draw()'s cross-thread early-out checks are reliable.
|
Merged with main again, but noticed that the changes pushed since last time still did not actually fully fix the deadlock issue, so this PR is still needed (I just updated the PR description again). |
3 participants
[#424] Fix: app.rive.runtime.kotlin.core.errors.RiveException: Accessing disposed C++ object RiveArtboardRenderer.
The crash is a TOCTOU race on the render worker:
draw()can callresizeArtboard()(which reads renderer width/height) while another thread is deleting the renderer, so the C++ pointer is disposed between thehasCppObjectcheck and the dimension read.Master partially addressed this in #11831 by splitting
resizeArtboard()into separate frameLock/fileLock sections, but resize still runs outsidedraw()'sframeLock.delete()can still win the race oncerequireArtboardResizeis cleared andresizeArtboard()begins.This PR keeps resize and draw under the same lock scope: acquire
frameLock, thenfileLock(matchingcontroller.advance()), run resize if needed, then draw.resizeArtboard()is private and assumes its caller already holds both locks. ThedisposeDependencies()fileLock override is removed; with the new ordering, it would invert lock acquisition relative todraw()and deadlock.RiveFileController.isActiveis marked@Volatileso the worker thread sees UI-thread deactivation promptly insidedraw().Tests
The old dispose-during-draw tests used
RiveFileControllerwithout aFile, sodraw()andactiveArtboardfell back to synchronizing on different objects. They passed even whenfileLockwas removed fromdraw(). Tests now loadR.raw.emptyso both paths shareFile.fileLock, matching productionRiveAnimationViewusage.deleteRendererDuringFrame—frameLockonly (no File)drawHoldsFileLockBlocksActiveArtboardMutation— UIactiveArtboardmutation blocks onfileLockdisposeArtboardDuringFrameAfterEnteringSyncBlock/BeforeEnteringSyncBlock— artboard disposal while draw holdsfileLockdeleteRendererBetweenCppObjectCheckAndDimensionRead_doesNotCrash— Crash! app.rive.runtime.kotlin.core.errors.RiveException: Accessing disposed C++ object RiveArtboardRenderer. #424 regression (delete during resize pause)Lock-order validation: removing
fileLockfromdraw()fails the file-lock tests; removingframeLockfails the frame-lock / #424 tests.Issue fixed: #424