Skip to content

Make RCTUnsafeExecuteOnMainQueueSync safer #51425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react-native/React/Base/RCTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ RCT_EXTERN void RCTExecuteOnMainQueue(dispatch_block_t block);
// Legacy function to execute the specified block on the main queue synchronously.
// Please do not use this unless you know what you're doing.
RCT_EXTERN void RCTUnsafeExecuteOnMainQueueSync(dispatch_block_t block);
RCT_EXTERN void RCTUnsafeExecuteOnMainQueueSyncWithError(dispatch_block_t block, NSString *context);
RCT_EXTERN void RCTUnsafeExecuteOnMainQueueSyncWithError(dispatch_block_t block, NSString *_Nullable context);

// Get screen metrics in a thread-safe way
RCT_EXTERN CGFloat RCTScreenScale(void);
Expand Down
70 changes: 43 additions & 27 deletions packages/react-native/React/Base/RCTUtils.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#import <CommonCrypto/CommonCrypto.h>

#import <React/RCTUtilsUIOverride.h>
#import <ReactCommon/RuntimeExecutorSyncUIThreadUtils.h>
#import "RCTAssert.h"
#import "RCTLog.h"

Expand Down Expand Up @@ -295,56 +296,71 @@ void RCTExecuteOnMainQueue(dispatch_block_t block)
}
}

static BOOL RCTIsJSThread()
{
return [[NSThread currentThread].name containsString:@"JavaScript"];
}

// Please do not use this method
// unless you know what you are doing.
void RCTUnsafeExecuteOnMainQueueSync(dispatch_block_t block)
{
if (RCTIsMainQueue()) {
block();
} else {
if (facebook::react::ReactNativeFeatureFlags::disableMainQueueSyncDispatchIOS()) {
RCTLogError(@"RCTUnsafeExecuteOnMainQueueSync: Sync dispatches to the main queue can deadlock React Native.");
}
dispatch_sync(dispatch_get_main_queue(), ^{
block();
});
}
RCTUnsafeExecuteOnMainQueueSyncWithError(block, nil);
}

// Please do not use this method
// unless you know what you are doing.
void RCTUnsafeExecuteOnMainQueueSyncWithError(dispatch_block_t block, NSString *context)
void RCTUnsafeExecuteOnMainQueueSyncWithError(dispatch_block_t block, NSString *_Nullable context)
{
if (RCTIsMainQueue()) {
block();
} else {
if (facebook::react::ReactNativeFeatureFlags::disableMainQueueSyncDispatchIOS()) {
RCTLogError(@"RCTUnsafeExecuteOnMainQueueSync: %@", context);
return;
}

if (facebook::react::ReactNativeFeatureFlags::enableSaferMainQueueSyncDispatchOnIOS()) {
if (RCTIsJSThread()) {
facebook::react::unsafeExecuteOnMainThreadSync(block);
return;
}
dispatch_sync(dispatch_get_main_queue(), ^{
block();
});
} else if (facebook::react::ReactNativeFeatureFlags::disableMainQueueSyncDispatchIOS()) {
RCTLogError(
@"RCTUnsafeExecuteOnMainQueueSync: %@",
context ?: @"Sync dispatches to the main queue can deadlock React Native.");
}

dispatch_sync(dispatch_get_main_queue(), ^{
block();
});
}

static void RCTUnsafeExecuteOnMainQueueOnceSync(dispatch_once_t *onceToken, dispatch_block_t block)
{
// The solution was borrowed from a post by Sophie Alpert:
// https://sophiebits.com/2014/04/02/dispatch-once-initialization-on-the-main-thread
// See also: https://www.mikeash.com/pyblog/friday-qa-2014-06-06-secrets-of-dispatch_once.html
if (RCTIsMainQueue()) {
auto executeOnce = ^{
dispatch_once(onceToken, block);
} else {
if (DISPATCH_EXPECT(*onceToken == 0L, NO)) {
if (facebook::react::ReactNativeFeatureFlags::disableMainQueueSyncDispatchIOS()) {
RCTLogError(
@"RCTUnsafeExecuteOnMainQueueOnceSync: Sync dispatches to the main queue can deadlock React Native.");
}
dispatch_sync(dispatch_get_main_queue(), ^{
dispatch_once(onceToken, block);
});
};

if (RCTIsMainQueue()) {
executeOnce();
return;
}

if (!DISPATCH_EXPECT(*onceToken == 0L, NO)) {
return;
}

if (facebook::react::ReactNativeFeatureFlags::enableSaferMainQueueSyncDispatchOnIOS()) {
if (RCTIsJSThread()) {
facebook::react::unsafeExecuteOnMainThreadSync(block);
return;
}
} else if (facebook::react::ReactNativeFeatureFlags::disableMainQueueSyncDispatchIOS()) {
RCTLogError(@"RCTUnsafeExecuteOnMainQueueOnceSync: Sync dispatches to the main queue can deadlock React Native.");
}

dispatch_sync(dispatch_get_main_queue(), executeOnce);
}

CGFloat RCTScreenScale(void)
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<<e9a109fd77667dd0cb945ef6ef9737f2>>
* @generated SignedSource<<24415ceea0dad0fec42af0b77ceec393>>
*/

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

/**
* Make RCTUnsafeExecuteOnMainQueueSync less likely to deadlock, when used in conjuction with sync rendering/events.
*/
@JvmStatic
public fun enableSaferMainQueueSyncDispatchOnIOS(): Boolean = accessor.enableSaferMainQueueSyncDispatchOnIOS()

/**
* Dispatches state updates synchronously in Fabric (e.g.: updates the scroll position in the shadow tree synchronously from the main thread).
*/
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<<1a5cd689229d4e0c31070123045e84da>>
* @generated SignedSource<<07f351d4f8971f7bd7d2eb0115760565>>
*/

/**
Expand Down Expand Up @@ -48,6 +48,7 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
private var enablePreparedTextLayoutCache: Boolean? = null
private var enablePropsUpdateReconciliationAndroidCache: Boolean? = null
private var enableResourceTimingAPICache: Boolean? = null
private var enableSaferMainQueueSyncDispatchOnIOSCache: Boolean? = null
private var enableSynchronousStateUpdatesCache: Boolean? = null
private var enableViewCullingCache: Boolean? = null
private var enableViewRecyclingCache: Boolean? = null
Expand Down Expand Up @@ -321,6 +322,15 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
return cached
}

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

override fun enableSynchronousStateUpdates(): Boolean {
var cached = enableSynchronousStateUpdatesCache
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<<c39d0c1834797b3309b4fc5ce814280c>>
* @generated SignedSource<<db670241f820223af2783a44ca452d98>>
*/

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

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

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

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

@DoNotStrip @JvmStatic public external fun enableViewCulling(): 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<<7f357475254104729cc7910c14e1c1fb>>
* @generated SignedSource<<c7599024bfc8c04b5d53145d16a06c20>>
*/

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

override fun enableResourceTimingAPI(): Boolean = false

override fun enableSaferMainQueueSyncDispatchOnIOS(): Boolean = false

override fun enableSynchronousStateUpdates(): Boolean = false

override fun enableViewCulling(): 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<<14cd1a58bd153dedda045a72c1494caa>>
* @generated SignedSource<<52f72deb1601f741295b22caa9755749>>
*/

/**
Expand Down Expand Up @@ -52,6 +52,7 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
private var enablePreparedTextLayoutCache: Boolean? = null
private var enablePropsUpdateReconciliationAndroidCache: Boolean? = null
private var enableResourceTimingAPICache: Boolean? = null
private var enableSaferMainQueueSyncDispatchOnIOSCache: Boolean? = null
private var enableSynchronousStateUpdatesCache: Boolean? = null
private var enableViewCullingCache: Boolean? = null
private var enableViewRecyclingCache: Boolean? = null
Expand Down Expand Up @@ -353,6 +354,16 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
return cached
}

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

override fun enableSynchronousStateUpdates(): Boolean {
var cached = enableSynchronousStateUpdatesCache
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<<593b1d64dc31038140032a6b0a439700>>
* @generated SignedSource<<8c2ffe69841478213407e92e44519157>>
*/

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

@DoNotStrip public fun enableResourceTimingAPI(): Boolean

@DoNotStrip public fun enableSaferMainQueueSyncDispatchOnIOS(): Boolean

@DoNotStrip public fun enableSynchronousStateUpdates(): Boolean

@DoNotStrip public fun enableViewCulling(): 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<<d74dc8182a14fddbd7118149298515bd>>
* @generated SignedSource<<e197b252b246d3c777ce1850d365220e>>
*/

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

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

bool enableSynchronousStateUpdates() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("enableSynchronousStateUpdates");
Expand Down Expand Up @@ -471,6 +477,11 @@ bool JReactNativeFeatureFlagsCxxInterop::enableResourceTimingAPI(
return ReactNativeFeatureFlags::enableResourceTimingAPI();
}

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

bool JReactNativeFeatureFlagsCxxInterop::enableSynchronousStateUpdates(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::enableSynchronousStateUpdates();
Expand Down Expand Up @@ -686,6 +697,9 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"enableResourceTimingAPI",
JReactNativeFeatureFlagsCxxInterop::enableResourceTimingAPI),
makeNativeMethod(
"enableSaferMainQueueSyncDispatchOnIOS",
JReactNativeFeatureFlagsCxxInterop::enableSaferMainQueueSyncDispatchOnIOS),
makeNativeMethod(
"enableSynchronousStateUpdates",
JReactNativeFeatureFlagsCxxInterop::enableSynchronousStateUpdates),
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<<a5ab0022f6a01bd6e929d36d9d87db10>>
* @generated SignedSource<<7f021510ca3f485b75e231d18cb58db9>>
*/

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

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

static bool enableSynchronousStateUpdates(
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<<88fdbea2f97f628187164a47a9737da0>>
* @generated SignedSource<<997a6c9e018988ae1e4b7f02bad8f433>>
*/

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

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

bool ReactNativeFeatureFlags::enableSynchronousStateUpdates() {
return getAccessor().enableSynchronousStateUpdates();
}
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<<a4123bc6f44835c022a1a5238908674c>>
* @generated SignedSource<<b4febb028accc1620853ee78403a707a>>
*/

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

/**
* Make RCTUnsafeExecuteOnMainQueueSync less likely to deadlock, when used in conjuction with sync rendering/events.
*/
RN_EXPORT static bool enableSaferMainQueueSyncDispatchOnIOS();

/**
* Dispatches state updates synchronously in Fabric (e.g.: updates the scroll position in the shadow tree synchronously from the main thread).
*/
Expand Down
Loading
Loading