Fix 49958: Close a view leak due to lossy onAnimationEnd callback #49959
Conversation
Android's onAnimationEnd callback is lossy and ocasionally just does not fire. However the LayoutAnimationController maintains a sparse array of animations (with Strong View refs) that is only cleaned when the onAnimationEnd callback is invoked. This results in a leak of Android View objects over time. To avoid this, the Strong View refs are migrated to WeakReference's and the associated sparse array is cleaned of any invalid layout animations in response to the reset() call. This closes two leaks: 1. Unbound growth in LayoutAnimationController::mLayoutHandlers 2. Pinning View objects into memory as the sole remaining GC root
facebook-github-bot
added
CLA Signed
| for (int i = mLayoutHandlers.size() - 1; i >= 0; i--) { | ||
| LayoutHandlingAnimation animation = mLayoutHandlers.valueAt(i); | ||
| if (!animation.isValid()) { | ||
| mLayoutHandlers.removeAt(i); | ||
| } | ||
| } |
There was a problem hiding this comment.
Why shouldn't we fully clear out mLayoutHandlers here?
There was a problem hiding this comment.
That was my initial thought as well, but the persistence of Animations across UI operations batch runs (one source of call chain to reset()) almost looks intentional. Because of that, I was worried about unintentionally breaking a non-obvious, yet intentional path in the original implementation.
This down-scoped cleanup will allow Animation objects to persist longer than their useful time but should be safe. A more complete cleanup would remove the Animation objects sooner but at some risk of regression.
|
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary:
Android's onAnimationEnd callback is lossy and ocasionally just does not fire. However the LayoutAnimationController maintains a sparse array of animations (with Strong View refs) that is only cleaned when the onAnimationEnd callback is invoked. This results in a leak of Android View objects over time.
To avoid this, the Strong View refs are migrated to WeakReference's and the associated sparse array is cleaned of any invalid layout animations in response to the reset() call.
This closes two leaks:
Changelog:
Pick one each for the category and type tags:
[ANDROID] [Fixed]- Fixes memory leak
Test Plan