fix(android): don't re-acquire a disposed Rive file on view re-attach#318
fix(android): don't re-acquire a disposed Rive file on view re-attach#318mfazekas wants to merge 3 commits into
Conversation
After a RiveView is dropped, dispose() releases the controller (which releases its File), but rendererAttributes.resource still caches a ResourceRiveFile pointing at the now-disposed File. If the Android view is later attached to a window again (e.g. Fabric view recycling), RiveAnimationView.onAttachedToWindow reloads that stale resource and crashes with "Cannot acquire a disposed object" in RiveFileController.setFile -> NativeObject.acquire. Legacy backend: null rendererAttributes.resource in dispose() (mirrors RiveAnimationView.saveControllerState()) and drop a stale dead-file resource in onAttachedToWindow before calling super. New backend has no cached-resource reload path; null riveWorker in dispose() so a re-attach can't create a surface on a released command queue. Reproduced and verified with the new Disposed File Re-attach reproducer page, backed by a dev-only RiveViewReattach native module in the example app that re-attaches the dropped Android view like view recycling would: FAIL (crash) before the fix, PASS after.
…ive helper The reproducer page is reachable from the Expo examples via the shared PagesList, but the RiveViewReattach helper only exists in the bare example app — pressing Run there would throw.
…tils nitro package Private workspace package (not published) with an Android-only Nitro HybridObject exposing captureView/reattachCapturedView/releaseCapturedView. Autolinked into all three example apps (bare + both Expo apps via prebuild), replacing the bare-example-only RiveViewReattach bridge module, so the Disposed File Re-attach reproducer works everywhere.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
ktlint
🚫 [ktlint] standard:wrapping reported by reviewdog 🐶
Missing newline after ","
🚫 [ktlint] standard:wrapping reported by reviewdog 🐶
Missing newline before ")"
🚫 [ktlint] standard:argument-list-wrapping reported by reviewdog 🐶
Argument should be on a separate line (unless all arguments can fit a single line)
🚫 [ktlint] standard:argument-list-wrapping reported by reviewdog 🐶
Missing newline before ")"
| @@ -0,0 +1,63 @@ | |||
| /// | |||
There was a problem hiding this comment.
🚫 [ktlint] standard:comment-spacing reported by reviewdog 🐶
Missing space after //
| @@ -0,0 +1,63 @@ | |||
| /// | |||
| /// HybridDebugUtilsSpec.kt | |||
There was a problem hiding this comment.
🚫 [ktlint] standard:comment-spacing reported by reviewdog 🐶
Missing space after //
| @@ -0,0 +1,63 @@ | |||
| /// | |||
| /// HybridDebugUtilsSpec.kt | |||
| /// This file was generated by nitrogen. DO NOT MODIFY THIS FILE. | |||
There was a problem hiding this comment.
🚫 [ktlint] standard:comment-spacing reported by reviewdog 🐶
Missing space after //
| /// | ||
| /// HybridDebugUtilsSpec.kt | ||
| /// This file was generated by nitrogen. DO NOT MODIFY THIS FILE. | ||
| /// https://github.com/mrousavy/nitro |
There was a problem hiding this comment.
🚫 [ktlint] standard:comment-spacing reported by reviewdog 🐶
Missing space after //
| /// HybridDebugUtilsSpec.kt | ||
| /// This file was generated by nitrogen. DO NOT MODIFY THIS FILE. | ||
| /// https://github.com/mrousavy/nitro | ||
| /// Copyright © Marc Rousavy @ Margelo |
There was a problem hiding this comment.
🚫 [ktlint] standard:comment-spacing reported by reviewdog 🐶
Missing space after //
| companion object { | ||
| private const val TAG = "RiveDebugUtilsOnLoad" | ||
| private var didLoad = false | ||
| /** |
There was a problem hiding this comment.
🚫 [ktlint] standard:spacing-between-declarations-with-comments reported by reviewdog 🐶
Declarations and declarations with comments should have an empty space between.
| Log.i(TAG, "Successfully loaded RiveDebugUtils C++ library!") | ||
| didLoad = true | ||
| } catch (e: Error) { | ||
| Log.e(TAG, "Failed to load RiveDebugUtils C++ library! Is it properly installed and linked? " + |
There was a problem hiding this comment.
🚫 [ktlint] standard:wrapping reported by reviewdog 🐶
Missing newline after "("
| Log.i(TAG, "Successfully loaded RiveDebugUtils C++ library!") | ||
| didLoad = true | ||
| } catch (e: Error) { | ||
| Log.e(TAG, "Failed to load RiveDebugUtils C++ library! Is it properly installed and linked? " + |
There was a problem hiding this comment.
🚫 [ktlint] standard:argument-list-wrapping reported by reviewdog 🐶
Argument should be on a separate line (unless all arguments can fit a single line)
| Log.i(TAG, "Successfully loaded RiveDebugUtils C++ library!") | ||
| didLoad = true | ||
| } catch (e: Error) { | ||
| Log.e(TAG, "Failed to load RiveDebugUtils C++ library! Is it properly installed and linked? " + |
There was a problem hiding this comment.
🚫 [ktlint] standard:wrapping reported by reviewdog 🐶
Missing newline after ","
| Log.i(TAG, "Successfully loaded RiveDebugUtils C++ library!") | ||
| didLoad = true | ||
| } catch (e: Error) { | ||
| Log.e(TAG, "Failed to load RiveDebugUtils C++ library! Is it properly installed and linked? " + |
There was a problem hiding this comment.
🚫 [ktlint] standard:argument-list-wrapping reported by reviewdog 🐶
Argument should be on a separate line (unless all arguments can fit a single line)
1 participant
Fixes a production crash: after a RiveView is dropped,
dispose()releases the controller (which releases itsFile), butrendererAttributes.resourcestill caches aResourceRiveFilepointing at the now-disposed file. If the Android view is later attached to a window again (e.g. Fabric view recycling),RiveAnimationView.onAttachedToWindowreloads that stale resource and throwsCannot acquire a disposed objectinRiveFileController.setFile->NativeObject.acquire.Legacy backend:
dispose()now nullsrendererAttributes.resourcebefore releasing the controller (mirrorsRiveAnimationView.saveControllerState()), andonAttachedToWindowdrops a cachedResourceRiveFilewhose file has no cpp object before calling super. The new (CommandQueue) backend has no cached-resource reload path; itsdispose()now also nullsriveWorkerso a re-attach can't create a surface on a released command queue.Reproduced and verified with the included
Disposed File Re-attachreproducer page. The re-attach itself is driven byrive-debug-utils, a new private workspace package with an Android-only Nitro hybrid object that captures the dropped Android view and re-attaches it the way view recycling would; it autolinks into all three example apps. The reproducer crashes with the exact production stack before the fix and passes after.