Fix 49958: Close a view leak due to lossy onAnimationEnd callback (#49959)

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:
1. Unbound growth in LayoutAnimationController::mLayoutHandlers
2. Pinning View objects into memory as the sole remaining GC root

## Changelog:

1. Made OpacityAnimation and PositionAndSizeAnimation classes hold weak refs to views only
2. Added a method to LayoutHandlingAnimation to surface if their view ref is gone
3. Added cleanup for Animation with bad view refs

Pick one each for the category and type tags:

[ANDROID] [Fixed]- Fixes memory leak

Pull Request resolved: https://github.com/facebook/react-native/pull/49959

Test Plan: * Primarily code inspection and regression given the intermittent nature of Android's failure to execute the callback.

Reviewed By: alanleedev

Differential Revision: D71037262

Pulled By: javache

fbshipit-source-id: 1fa4eaa2ca839f347a55cb37e2648db972748586
This commit is contained in:
Andrew Knapp
2025-03-13 04:17:17 -07:00
committed by Facebook GitHub Bot
parent e2eed3984e
commit 313d7d79d4
4 changed files with 66 additions and 38 deletions
@@ -87,6 +87,12 @@ public class LayoutAnimationController {
mCompletionRunnable = null;
mShouldAnimateLayout = false;
mMaxAnimationDuration = -1;
for (int i = mLayoutHandlers.size() - 1; i >= 0; i--) {
LayoutHandlingAnimation animation = mLayoutHandlers.valueAt(i);
if (!animation.isValid()) {
mLayoutHandlers.removeAt(i);
}
}
}
public boolean shouldAnimateLayout(View viewToAnimate) {
@@ -121,8 +127,12 @@ public class LayoutAnimationController {
// the existing animation would still animate to the old layout.
LayoutHandlingAnimation existingAnimation = mLayoutHandlers.get(reactTag);
if (existingAnimation != null) {
existingAnimation.onLayoutUpdate(x, y, width, height);
return;
if (!existingAnimation.isValid()) {
mLayoutHandlers.remove(reactTag);
} else {
existingAnimation.onLayoutUpdate(x, y, width, height);
return;
}
}
// Determine which animation to use : if view is initially invisible, use create animation,
@@ -23,4 +23,13 @@ internal interface LayoutHandlingAnimation {
* @param height the new height value for the view
*/
fun onLayoutUpdate(x: Int, y: Int, width: Int, height: Int)
/**
* Returns whether the animation is valid and should be used. Because layout animations generally
* hold {@link java.lang.ref.WeakReference<android.view.View>} objects, it's possible that the
* view has been garbage collected. In this case, the animation should not be used.
*
* @return true if the animation is valid and can be used, false otherwise
*/
fun isValid(): Boolean
}
@@ -14,17 +14,16 @@ import com.facebook.react.common.annotations.VisibleForTesting
import com.facebook.react.common.annotations.internal.LegacyArchitecture
import com.facebook.react.common.annotations.internal.LegacyArchitectureLogLevel
import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger
import java.lang.ref.WeakReference
/**
* Animation responsible for updating opacity of a view. It should ideally use hardware texture to
* optimize rendering performances.
*/
@LegacyArchitecture
internal class OpacityAnimation(
private val view: View,
private val startOpacity: Float,
endOpacity: Float
) : Animation() {
internal class OpacityAnimation(view: View, private val startOpacity: Float, endOpacity: Float) :
Animation() {
private val viewRef = WeakReference(view)
private val deltaOpacity = endOpacity - startOpacity
init {
@@ -33,19 +32,24 @@ internal class OpacityAnimation(
"OpacityAnimation", LegacyArchitectureLogLevel.WARNING)
}
class OpacityAnimationListener(private val view: View) : Animation.AnimationListener {
class OpacityAnimationListener(view: View) : Animation.AnimationListener {
private val viewRef = WeakReference(view)
private var layerTypeChanged = false
override fun onAnimationStart(animation: Animation) {
if (view.hasOverlappingRendering() && view.layerType == View.LAYER_TYPE_NONE) {
layerTypeChanged = true
view.setLayerType(View.LAYER_TYPE_HARDWARE, null)
viewRef.get()?.let { view ->
if (view.hasOverlappingRendering() && view.layerType == View.LAYER_TYPE_NONE) {
layerTypeChanged = true
view.setLayerType(View.LAYER_TYPE_HARDWARE, null)
}
}
}
override fun onAnimationEnd(animation: Animation) {
if (layerTypeChanged) {
view.setLayerType(View.LAYER_TYPE_NONE, null)
viewRef.get()?.let { view ->
if (layerTypeChanged) {
view.setLayerType(View.LAYER_TYPE_NONE, null)
}
}
}
@@ -56,7 +60,7 @@ internal class OpacityAnimation(
@VisibleForTesting
public override fun applyTransformation(interpolatedTime: Float, t: Transformation) {
view.alpha = startOpacity + deltaOpacity * interpolatedTime
viewRef.get()?.let { view -> view.alpha = startOpacity + deltaOpacity * interpolatedTime }
}
override fun willChangeBounds(): Boolean {
@@ -12,6 +12,7 @@ import android.view.animation.Animation
import android.view.animation.Transformation
import com.facebook.react.common.annotations.internal.LegacyArchitecture
import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger
import java.lang.ref.WeakReference
/**
* Animation responsible for updating size and position of a view. We can't use scaling as view
@@ -20,13 +21,9 @@ import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger
* ScaleAnimation and TranslateAnimation.
*/
@LegacyArchitecture
internal class PositionAndSizeAnimation(
private val view: View,
x: Int,
y: Int,
width: Int,
height: Int
) : Animation(), LayoutHandlingAnimation {
internal class PositionAndSizeAnimation(view: View, x: Int, y: Int, width: Int, height: Int) :
Animation(), LayoutHandlingAnimation {
private val viewRef = WeakReference(view)
private var startX = 0f
private var startY = 0f
private var deltaX = 0f
@@ -41,15 +38,17 @@ internal class PositionAndSizeAnimation(
}
override fun applyTransformation(interpolatedTime: Float, t: Transformation) {
val newX = startX + deltaX * interpolatedTime
val newY = startY + deltaY * interpolatedTime
val newWidth = startWidth + deltaWidth * interpolatedTime
val newHeight = startHeight + deltaHeight * interpolatedTime
view.layout(
Math.round(newX),
Math.round(newY),
Math.round(newX + newWidth),
Math.round(newY + newHeight))
viewRef.get()?.let { view ->
val newX = startX + deltaX * interpolatedTime
val newY = startY + deltaY * interpolatedTime
val newWidth = startWidth + deltaWidth * interpolatedTime
val newHeight = startHeight + deltaHeight * interpolatedTime
view.layout(
Math.round(newX),
Math.round(newY),
Math.round(newX + newWidth),
Math.round(newY + newHeight))
}
}
override fun onLayoutUpdate(x: Int, y: Int, width: Int, height: Int) {
@@ -58,20 +57,26 @@ internal class PositionAndSizeAnimation(
calculateAnimation(x, y, width, height)
}
override fun isValid(): Boolean {
return viewRef.get() != null
}
override fun willChangeBounds(): Boolean {
return true
}
private fun calculateAnimation(x: Int, y: Int, width: Int, height: Int) {
startX = view.x - view.translationX
startY = view.y - view.translationY
startWidth = view.width
startHeight = view.height
viewRef.get()?.let { view ->
startX = view.x - view.translationX
startY = view.y - view.translationY
startWidth = view.width
startHeight = view.height
deltaX = x - startX
deltaY = y - startY
deltaWidth = width - startWidth
deltaHeight = height - startHeight
deltaX = x - startX
deltaY = y - startY
deltaWidth = width - startWidth
deltaHeight = height - startHeight
}
}
private companion object {