From 2a18d83521b30642a7e403fd55a5afd4ef2d8cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Knutzen?= <2263015+hakonk@users.noreply.github.com> Date: Mon, 27 Jan 2025 09:20:47 -0800 Subject: [PATCH] Make RCTDeviceInfo._invalidated std::atomic (#48890) Summary: When running the tests associated with `RNTestPods` with `TSan` enabled, I get a data race: ``` WARNING: ThreadSanitizer: data race (pid=28047) Read of size 1 at 0x000144c30be9 by thread T32: #0 -[RCTDeviceInfo invalidate] (RNTesterUnitTests:arm64+0x434bf8) https://github.com/facebook/react-native/issues/1 __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ (CoreFoundation:arm64+0x5e7fc) https://github.com/facebook/react-native/issues/2 decltype(std::declval()()) std::__1::__invoke[abi:de180100](&&, decltype(std::declval()())&&...) (RNTesterUnitTests:arm64+0x2a19d4) https://github.com/facebook/react-native/issues/3 std::__1::__function::__func, void ()>::operator()() (RNTesterUnitTests:arm64+0x2a16ec) https://github.com/facebook/react-native/issues/4 std::__1::__function::__value_func::operator()[abi:de180100]() const (RNTesterUnitTests:arm64+0x2455d4) https://github.com/facebook/react-native/issues/5 std::__1::function::operator()() const (RNTesterUnitTests:arm64+0x245404) https://github.com/facebook/react-native/issues/6 facebook::react::tryAndReturnError(std::__1::function const&) (RNTesterUnitTests:arm64+0x2c85b4) https://github.com/facebook/react-native/issues/7 -[RCTCxxBridge _tryAndHandleError:] (RNTesterUnitTests:arm64+0x27c9e8) https://github.com/facebook/react-native/issues/8 __NSThreadPerformPerform (Foundation:arm64+0x76c5e8) https://github.com/facebook/react-native/issues/9 __NSThread__start__ (Foundation:arm64+0x76c27c) Previous write of size 1 at 0x000144c30be9 by thread T30: #0 -[RCTDeviceInfo invalidate] (RNTesterUnitTests:arm64+0x434c1c) https://github.com/facebook/react-native/issues/1 __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ (CoreFoundation:arm64+0x5e7fc) https://github.com/facebook/react-native/issues/2 decltype(std::declval()()) std::__1::__invoke[abi:de180100](&&, decltype(std::declval()())&&...) (RNTesterUnitTests:arm64+0x2a19d4) https://github.com/facebook/react-native/issues/3 std::__1::__function::__func, void ()>::operator()() (RNTesterUnitTests:arm64+0x2a16ec) https://github.com/facebook/react-native/issues/4 std::__1::__function::__value_func::operator()[abi:de180100]() const (RNTesterUnitTests:arm64+0x2455d4) https://github.com/facebook/react-native/issues/5 std::__1::function::operator()() const (RNTesterUnitTests:arm64+0x245404) https://github.com/facebook/react-native/issues/6 facebook::react::tryAndReturnError(std::__1::function const&) (RNTesterUnitTests:arm64+0x2c85b4) https://github.com/facebook/react-native/issues/7 -[RCTCxxBridge _tryAndHandleError:] (RNTesterUnitTests:arm64+0x27c9e8) https://github.com/facebook/react-native/issues/8 __NSThreadPerformPerform (Foundation:arm64+0x76c5e8) https://github.com/facebook/react-native/issues/9 __NSThread__start__ (Foundation:arm64+0x76c27c) Location is heap block of size 48 at 0x000144c30bd0 allocated by main thread: #0 calloc (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x53b90) https://github.com/facebook/react-native/issues/1 _malloc_type_calloc_outlined (libsystem_malloc.dylib:arm64+0xf8dc) https://github.com/facebook/react-native/issues/2 -[RCTModuleData setUpInstanceAndBridge:] (RNTesterUnitTests:arm64+0x32715c) https://github.com/facebook/react-native/issues/3 __25-[RCTModuleData instance]_block_invoke (RNTesterUnitTests:arm64+0x32a288) https://github.com/facebook/react-native/issues/4 RCTUnsafeExecuteOnMainQueueSync (RNTesterUnitTests:arm64+0x3d6e1c) https://github.com/facebook/react-native/issues/5 -[RCTModuleData instance] (RNTesterUnitTests:arm64+0x329d78) https://github.com/facebook/react-native/issues/6 __49-[RCTCxxBridge _prepareModulesWithDispatchGroup:]_block_invoke (RNTesterUnitTests:arm64+0x287e74) https://github.com/facebook/react-native/issues/7 __wrap_dispatch_group_async_block_invoke (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x7cffc) https://github.com/facebook/react-native/issues/8 _dispatch_client_callout (libdispatch.dylib:arm64+0x3c04) https://github.com/facebook/react-native/issues/9 __70-[XCTestCase _shouldContinueAfterPerformingSetUpSequenceWithSelector:]_block_invoke.136 (XCTestCore:arm64+0x2d068) Thread T32 (tid=4154385, running) created by main thread at: #0 pthread_create (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x3027c) https://github.com/facebook/react-native/issues/1 -[NSThread startAndReturnError:] (Foundation:arm64+0x76bec4) https://github.com/facebook/react-native/issues/2 -[RCTBridge setUp] (RNTesterUnitTests:arm64+0x23d638) https://github.com/facebook/react-native/issues/3 -[RCTBridge initWithDelegate:bundleURL:moduleProvider:launchOptions:] (RNTesterUnitTests:arm64+0x23addc) https://github.com/facebook/react-native/issues/4 -[RCTBridge initWithBundleURL:moduleProvider:launchOptions:] (RNTesterUnitTests:arm64+0x23aa4c) https://github.com/facebook/react-native/issues/5 -[RCTImageLoaderTests testImageLoaderUsesImageDecoderWithHighestPriority] (RNTesterUnitTests:arm64+0xc578) https://github.com/facebook/react-native/issues/6 __invoking___ (CoreFoundation:arm64+0x132cdc) Thread T30 (tid=4154383, running) created by main thread at: #0 pthread_create (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x3027c) https://github.com/facebook/react-native/issues/1 -[NSThread startAndReturnError:] (Foundation:arm64+0x76bec4) https://github.com/facebook/react-native/issues/2 -[RCTBridge setUp] (RNTesterUnitTests:arm64+0x23d638) https://github.com/facebook/react-native/issues/3 -[RCTBridge initWithDelegate:bundleURL:moduleProvider:launchOptions:] (RNTesterUnitTests:arm64+0x23addc) https://github.com/facebook/react-native/issues/4 -[RCTBridge initWithBundleURL:moduleProvider:launchOptions:] (RNTesterUnitTests:arm64+0x23aa4c) https://github.com/facebook/react-native/issues/5 -[RCTImageLoaderTests testImageDecoding] (RNTesterUnitTests:arm64+0xa8d0) https://github.com/facebook/react-native/issues/6 __invoking___ (CoreFoundation:arm64+0x132cdc) ``` The proposed solution is making the `BOOL` ivar in question a `std::atomic` instead. ## Changelog: [IOS][FIXED] Data race related to read/write of RCTDeviceInfo._invalidated. Pull Request resolved: https://github.com/facebook/react-native/pull/48890 Test Plan: Existing tests in `RNTesterPods` and manually running RNTester application. Reviewed By: christophpurrer Differential Revision: D68629011 Pulled By: javache fbshipit-source-id: 229d0db4aa13253b96ce0a20c9795c17e344cfc1 --- packages/react-native/React/CoreModules/RCTDeviceInfo.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native/React/CoreModules/RCTDeviceInfo.mm b/packages/react-native/React/CoreModules/RCTDeviceInfo.mm index 95395e190cf..302676157f7 100644 --- a/packages/react-native/React/CoreModules/RCTDeviceInfo.mm +++ b/packages/react-native/React/CoreModules/RCTDeviceInfo.mm @@ -16,6 +16,7 @@ #import #import #import +#import #import "CoreModulesPlugins.h" @@ -28,7 +29,7 @@ using namespace facebook::react; UIInterfaceOrientation _currentInterfaceOrientation; NSDictionary *_currentInterfaceDimensions; BOOL _isFullscreen; - BOOL _invalidated; + std::atomic _invalidated; } @synthesize moduleRegistry = _moduleRegistry; @@ -96,10 +97,9 @@ RCT_EXPORT_MODULE() - (void)invalidate { - if (_invalidated) { + if (_invalidated.exchange(YES)) { return; } - _invalidated = YES; [self _cleanupObservers]; }