Repository navigation
Conversation
RiveImage now releases its native image on dispose (with a finalizer fallback) and reports its decoded size so Hermes collects dropped images. Referenced assets are decoded in place instead of leaking a wrapper ref, a view detached before dispose() still releases its controller and File, and RiveFile releases its File on the main thread so it can't be freed under an in-flight asset set. Adds an AndroidMemoryLeak reproducer page.
e0fd8c0 to
a62f041
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The asset loader can race with image disposal, allowing the setter to receive a disposed image or freed native pointer.
3 open issues
What changed in this PR
Addresses native image and view memory leaks in the legacy Android backend by releasing owned references and improving disposal timing. The separate view-model reference leak remains outside this PR’s scope.
Changes:
- Release native images explicitly and on finalization, and report decoded memory usage.
- Avoid temporary asset-wrapper leaks and release detached views and files safely.
- Add a memory-leak reproducer and bound-image disposal harness tests.
| File | Description |
|---|---|
example/src/reproducers/AndroidMemoryLeak.tsx |
Adds mount/unmount and image-swap scenarios for manual memory checks. |
example/__tests__/image-dispose.harness.ts |
Tests image disposal and replacement while bound. |
android/src/legacy/java/com/rive/RiveReactNativeView.kt |
Releases detached view resources at most once. |
android/src/legacy/java/com/margelo/nitro/rive/ReferencedAssetLoader.kt |
Decodes assets directly to avoid temporary wrapper leaks. |
android/src/legacy/java/com/margelo/nitro/rive/HybridRiveImageFactory.kt |
Estimates decoded image memory usage. |
android/src/legacy/java/com/margelo/nitro/rive/HybridRiveImage.kt |
Releases native image references and reports memory size. |
android/src/legacy/java/com/margelo/nitro/rive/HybridRiveFile.kt |
Moves native file release to the main thread. |
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Native reference-count destruction depends on JVM finalization and Fabric detach/dispose ordering, requiring Android runtime validation of GC-driven memory reclamation and rapid unmounts beyond the added harness checks.
No open issues 🎉
3 resolved since last review
Review effort: Balanced
HayesGordon
left a comment
There was a problem hiding this comment.
LGTM, something caught be my agent and worth validating:
HybridViewModelImageProperty.setstill readsrenderImagedirectly (line 22). SincerenderImageis now nullable, setting an already-disposed image silently clears the property instead of failing. Worth routing it throughwithRenderImagelike the asset loader, and throwing if the image was disposed:val img = image as? HybridRiveImage if (img == null) { viewModelImage.set(null); return } img.withRenderImage { viewModelImage.set(it) } ?: throw IllegalStateException("RiveImage has been disposed")That also keeps it consistent with the locking added in bb6c71c.
HayesGordon
left a comment
There was a problem hiding this comment.
LGTM, but lets add a removal condition to the file, or to the README, or somewhere, to delete once the minimum supported nitro-modules includes margelo/nitro#1824
On the new Android backend, every `ViewModelImageProperty.set()` decoded a new `ImageAsset` on the command server and never closed it, even when setting the same image again. Swapping images grew GPU memory from 304 MB to 4.8 GB over 211 swaps (2048×2048 PNGs, emulator). The handle is now closed right after `setImage` (the property keeps its own ref), and memory stays flat at ~311 MB. `set(undefined)` now clears the image instead of being ignored. Sets run on a single-threaded scope so they apply in call order. Before, a clear could be overtaken by an earlier set that was still decoding. Adds a harness test for set/clear. Memory isn't asserted automatically; use the `AndroidMemoryLeak` reproducer from #426 (image swap scenarios) to check.
Read the render image under its lock so a concurrent dispose can't release it mid-set.
|
@HayesGordon thanks, applied your suggestion in cad3058: The removal condition for margelo/nitro#1824 belongs to the Xcode 27 backport, so I added it on #434 (892d9d2): a comment in |


Fixes native memory that is never freed on the legacy Android backend when data-bound images are swapped or Rive views are mounted and unmounted repeatedly. Over a long session it grows until the app crashes. Measured with 2048×2048 PNGs (about 21 MB of GPU memory each) on an emulator.
RiveImage.dispose()did not release the nativeRiveRenderImage, and Nitro doesn't call Kotlindispose()on GC. It now releases indispose(), has a finalizer fallback, and reports the decoded size (w×h×4) asmemorySizeso Hermes collects dropped images. Consumers (image properties, image assets) hold their own ref, so a disposed image keeps showing where it's bound. Image swaps: 276 MB → 6.9 GB over 309 swaps before, flat at ~330 MB after.ReferencedAssetLoadercreated a temporary image/font/audio wrapper per asset and never released it, because rive-android takes its own ref. It now usesasset.decode(bytes), like rive-android's own loaders.RiveReactNativeView: Fabric sometimes detaches the view beforedispose()arrives (2–3 of ~50 unmounts), so the controller and its File were never released.dispose()now releases on the UI thread if the view is already detached, at most once. Before: 3 of 47 Files leaked; after: 140 of 140 freed.RiveFile.dispose()releases the File on the main thread, where the asset loader sets decoded assets, so it can't be freed during an in-flight set. Based on a patch from a user.Adds the
AndroidMemoryLeakreproducer page (mount/unmount and image swap scenarios; the test PNGs are 2048×2048 but compress to 15 KB) and a harness test covering dispose and re-set of bound images. The memory itself isn't asserted automatically; the reproducer is the check.Known remaining leak: rive-android keeps an extra native ref on every
ViewModelInstancecreated throughViewModel.create*Instance, so instances and the images bound to them outlive their File. #429 works around it for known-affected rive-android versions. Without it, clear image properties (set(undefined)) before unmount.