[#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.
2 participants
[#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.
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.
Remove override of disposeDependencies since it should not also be synchronized on the file lock.
Issue fixed: #424