Skip to content

Commit

Permalink
Fix SurfaceMountingManager leaking activity from stopped surfaces (#4…
Browse files Browse the repository at this point in the history
…4584)

Summary:
Pull Request resolved: #44584

Changelog: [Android][Fixed] Surfaces no longer leak activity once stopped

Reviewed By: javache

Differential Revision: D57367419

fbshipit-source-id: 7aa69256284f97679ebcc3309f2b74650ec3fb51
  • Loading branch information
Oleh Malanchuk authored and facebook-github-bot committed May 17, 2024
1 parent 044aadb commit af72108
Show file tree
Hide file tree
Showing 20 changed files with 133 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.facebook.react.fabric.events.EventEmitterWrapper;
import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor;
import com.facebook.react.fabric.mounting.mountitems.MountItem;
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
import com.facebook.react.modules.core.ReactChoreographer;
import com.facebook.react.touch.JSResponderHandler;
import com.facebook.react.uimanager.IViewGroupManager;
Expand Down Expand Up @@ -301,6 +302,9 @@ public void stopSurface() {
mRootViewManager = null;
mMountItemExecutor = null;
mThemedReactContext = null;
if (ReactNativeFeatureFlags.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak()) {
mRemoveDeleteTreeUIFrameCallback = null;
}
mOnViewAttachMountItems.clear();

if (ReactFeatureFlags.enableViewRecycling) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<c35ee97cf5c4b5f77cdd045341f2848b>>
* @generated SignedSource<<fbc717f14e387b92aea7cb71202cce95>>
*/

/**
Expand Down Expand Up @@ -94,6 +94,12 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun enableUIConsistency(): Boolean = accessor.enableUIConsistency()

/**
* Fixes a leak in SurfaceMountingManager.mRemoveDeleteTreeUIFrameCallback
*/
@JvmStatic
public fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean = accessor.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak()

/**
* Forces the mounting layer on Android to always batch mount items instead of dispatching them immediately. This might fix some crashes related to synchronous state updates, where some views dispatch state updates during mount.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<1b84e5fa96120a511db6f831afb73eab>>
* @generated SignedSource<<cfbb7879b105079fe76e37b0683f7083>>
*/

/**
Expand All @@ -31,6 +31,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var enableMicrotasksCache: Boolean? = null
private var enableSynchronousStateUpdatesCache: Boolean? = null
private var enableUIConsistencyCache: Boolean? = null
private var fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache: Boolean? = null
private var forceBatchingMountItemsOnAndroidCache: Boolean? = null
private var inspectorEnableCxxInspectorPackagerConnectionCache: Boolean? = null
private var inspectorEnableModernCDPRegistryCache: Boolean? = null
Expand Down Expand Up @@ -139,6 +140,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean {
var cached = fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak()
fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache = cached
}
return cached
}

override fun forceBatchingMountItemsOnAndroid(): Boolean {
var cached = forceBatchingMountItemsOnAndroidCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<2cd7ab4688ca2179ba3a19e9a062f695>>
* @generated SignedSource<<77b8e93114ed0cc2b6bdd9ee46e162c7>>
*/

/**
Expand Down Expand Up @@ -50,6 +50,8 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun enableUIConsistency(): Boolean

@DoNotStrip @JvmStatic public external fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean

@DoNotStrip @JvmStatic public external fun forceBatchingMountItemsOnAndroid(): Boolean

@DoNotStrip @JvmStatic public external fun inspectorEnableCxxInspectorPackagerConnection(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<a2c4f2c950a7b92163fdc6219864d6ca>>
* @generated SignedSource<<13a5d73dea53b8d4f16946cebc49e684>>
*/

/**
Expand Down Expand Up @@ -45,6 +45,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun enableUIConsistency(): Boolean = false

override fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean = false

override fun forceBatchingMountItemsOnAndroid(): Boolean = false

override fun inspectorEnableCxxInspectorPackagerConnection(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<48f657b282ef658ab7e9024f470d37ad>>
* @generated SignedSource<<67f1828f6a4cf9b2cf808be12d3fb06b>>
*/

/**
Expand Down Expand Up @@ -35,6 +35,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var enableMicrotasksCache: Boolean? = null
private var enableSynchronousStateUpdatesCache: Boolean? = null
private var enableUIConsistencyCache: Boolean? = null
private var fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache: Boolean? = null
private var forceBatchingMountItemsOnAndroidCache: Boolean? = null
private var inspectorEnableCxxInspectorPackagerConnectionCache: Boolean? = null
private var inspectorEnableModernCDPRegistryCache: Boolean? = null
Expand Down Expand Up @@ -154,6 +155,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean {
var cached = fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache
if (cached == null) {
cached = currentProvider.fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak()
accessedFeatureFlags.add("fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak")
fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache = cached
}
return cached
}

override fun forceBatchingMountItemsOnAndroid(): Boolean {
var cached = forceBatchingMountItemsOnAndroidCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<0f3d31f94f4bded41936fe4ecafbdd4a>>
* @generated SignedSource<<a6a65fe7d9b04671de407a03e1feb161>>
*/

/**
Expand Down Expand Up @@ -45,6 +45,8 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun enableUIConsistency(): Boolean

@DoNotStrip public fun fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(): Boolean

@DoNotStrip public fun forceBatchingMountItemsOnAndroid(): Boolean

@DoNotStrip public fun inspectorEnableCxxInspectorPackagerConnection(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<630ef8f9e65223c8d4c2b07bcc0ebadd>>
* @generated SignedSource<<ad6b81206d1f410cd6c8d9109bd340bd>>
*/

/**
Expand Down Expand Up @@ -105,6 +105,12 @@ class ReactNativeFeatureFlagsProviderHolder
return method(javaProvider_);
}

bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak");
return method(javaProvider_);
}

bool forceBatchingMountItemsOnAndroid() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("forceBatchingMountItemsOnAndroid");
Expand Down Expand Up @@ -212,6 +218,11 @@ bool JReactNativeFeatureFlagsCxxInterop::enableUIConsistency(
return ReactNativeFeatureFlags::enableUIConsistency();
}

bool JReactNativeFeatureFlagsCxxInterop::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak();
}

bool JReactNativeFeatureFlagsCxxInterop::forceBatchingMountItemsOnAndroid(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid();
Expand Down Expand Up @@ -302,6 +313,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"enableUIConsistency",
JReactNativeFeatureFlagsCxxInterop::enableUIConsistency),
makeNativeMethod(
"fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak",
JReactNativeFeatureFlagsCxxInterop::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak),
makeNativeMethod(
"forceBatchingMountItemsOnAndroid",
JReactNativeFeatureFlagsCxxInterop::forceBatchingMountItemsOnAndroid),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<39dde965f082a0dffbe00763e184d1db>>
* @generated SignedSource<<eb5c66cf6c5ceb117b7cad784cbe20a1>>
*/

/**
Expand Down Expand Up @@ -63,6 +63,9 @@ class JReactNativeFeatureFlagsCxxInterop
static bool enableUIConsistency(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool forceBatchingMountItemsOnAndroid(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<d176ed0be7b015e7b9444a0b7ac7f7ba>>
* @generated SignedSource<<5001001b2979f9b759ddac1b035842bf>>
*/

/**
Expand Down Expand Up @@ -65,6 +65,10 @@ bool ReactNativeFeatureFlags::enableUIConsistency() {
return getAccessor().enableUIConsistency();
}

bool ReactNativeFeatureFlags::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() {
return getAccessor().fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak();
}

bool ReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid() {
return getAccessor().forceBatchingMountItemsOnAndroid();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<0f7b95b5d42c879dabea8f7d53f9cc17>>
* @generated SignedSource<<4dd4bd2c61bff37de19a52b1bb60211c>>
*/

/**
Expand Down Expand Up @@ -92,6 +92,11 @@ class ReactNativeFeatureFlags {
*/
RN_EXPORT static bool enableUIConsistency();

/**
* Fixes a leak in SurfaceMountingManager.mRemoveDeleteTreeUIFrameCallback
*/
RN_EXPORT static bool fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak();

/**
* Forces the mounting layer on Android to always batch mount items instead of dispatching them immediately. This might fix some crashes related to synchronous state updates, where some views dispatch state updates during mount.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<165de70ff21543c949e57971f1a8ff07>>
* @generated SignedSource<<7766e3504d137c8dee703a23d776689c>>
*/

/**
Expand Down Expand Up @@ -227,6 +227,24 @@ bool ReactNativeFeatureFlagsAccessor::enableUIConsistency() {
return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak() {
auto flagValue = fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak_.load();

if (!flagValue.has_value()) {
// This block is not exclusive but it is not necessary.
// If multiple threads try to initialize the feature flag, we would only
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(11, "fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak");

flagValue = currentProvider_->fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak();
fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeak_ = flagValue;
}

return flagValue.value();
}

bool ReactNativeFeatureFlagsAccessor::forceBatchingMountItemsOnAndroid() {
auto flagValue = forceBatchingMountItemsOnAndroid_.load();

Expand All @@ -236,7 +254,7 @@ bool ReactNativeFeatureFlagsAccessor::forceBatchingMountItemsOnAndroid() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(11, "forceBatchingMountItemsOnAndroid");
markFlagAsAccessed(12, "forceBatchingMountItemsOnAndroid");

flagValue = currentProvider_->forceBatchingMountItemsOnAndroid();
forceBatchingMountItemsOnAndroid_ = flagValue;
Expand All @@ -254,7 +272,7 @@ bool ReactNativeFeatureFlagsAccessor::inspectorEnableCxxInspectorPackagerConnect
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(12, "inspectorEnableCxxInspectorPackagerConnection");
markFlagAsAccessed(13, "inspectorEnableCxxInspectorPackagerConnection");

flagValue = currentProvider_->inspectorEnableCxxInspectorPackagerConnection();
inspectorEnableCxxInspectorPackagerConnection_ = flagValue;
Expand All @@ -272,7 +290,7 @@ bool ReactNativeFeatureFlagsAccessor::inspectorEnableModernCDPRegistry() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(13, "inspectorEnableModernCDPRegistry");
markFlagAsAccessed(14, "inspectorEnableModernCDPRegistry");

flagValue = currentProvider_->inspectorEnableModernCDPRegistry();
inspectorEnableModernCDPRegistry_ = flagValue;
Expand All @@ -290,7 +308,7 @@ bool ReactNativeFeatureFlagsAccessor::lazyAnimationCallbacks() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(14, "lazyAnimationCallbacks");
markFlagAsAccessed(15, "lazyAnimationCallbacks");

flagValue = currentProvider_->lazyAnimationCallbacks();
lazyAnimationCallbacks_ = flagValue;
Expand All @@ -308,7 +326,7 @@ bool ReactNativeFeatureFlagsAccessor::preventDoubleTextMeasure() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(15, "preventDoubleTextMeasure");
markFlagAsAccessed(16, "preventDoubleTextMeasure");

flagValue = currentProvider_->preventDoubleTextMeasure();
preventDoubleTextMeasure_ = flagValue;
Expand All @@ -326,7 +344,7 @@ bool ReactNativeFeatureFlagsAccessor::useModernRuntimeScheduler() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(16, "useModernRuntimeScheduler");
markFlagAsAccessed(17, "useModernRuntimeScheduler");

flagValue = currentProvider_->useModernRuntimeScheduler();
useModernRuntimeScheduler_ = flagValue;
Expand All @@ -344,7 +362,7 @@ bool ReactNativeFeatureFlagsAccessor::useNativeViewConfigsInBridgelessMode() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(17, "useNativeViewConfigsInBridgelessMode");
markFlagAsAccessed(18, "useNativeViewConfigsInBridgelessMode");

flagValue = currentProvider_->useNativeViewConfigsInBridgelessMode();
useNativeViewConfigsInBridgelessMode_ = flagValue;
Expand All @@ -362,7 +380,7 @@ bool ReactNativeFeatureFlagsAccessor::useStateAlignmentMechanism() {
// be accessing the provider multiple times but the end state of this
// instance and the returned flag value would be the same.

markFlagAsAccessed(18, "useStateAlignmentMechanism");
markFlagAsAccessed(19, "useStateAlignmentMechanism");

flagValue = currentProvider_->useStateAlignmentMechanism();
useStateAlignmentMechanism_ = flagValue;
Expand Down

0 comments on commit af72108

Please sign in to comment.