Skip to content

Commit

Permalink
Make RCTDeviceInfo._invalidated std::atomic<BOOL> (#48890)
Browse files Browse the repository at this point in the history
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] <null> (RNTesterUnitTests:arm64+0x434bf8)
    #1 __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ <null> (CoreFoundation:arm64+0x5e7fc)
    #2 decltype(std::declval<void () block_pointer __strong&>()()) std::__1::__invoke[abi:de180100]<void () block_pointer __strong&>(&&, decltype(std::declval<void () block_pointer __strong&>()())&&...) <null> (RNTesterUnitTests:arm64+0x2a19d4)
    #3 std::__1::__function::__func<void () block_pointer __strong, std::__1::allocator<std::__1::allocator>, void ()>::operator()() <null> (RNTesterUnitTests:arm64+0x2a16ec)
    #4 std::__1::__function::__value_func<void ()>::operator()[abi:de180100]() const <null> (RNTesterUnitTests:arm64+0x2455d4)
    #5 std::__1::function<void ()>::operator()() const <null> (RNTesterUnitTests:arm64+0x245404)
    #6 facebook::react::tryAndReturnError(std::__1::function<void ()> const&) <null> (RNTesterUnitTests:arm64+0x2c85b4)
    #7 -[RCTCxxBridge _tryAndHandleError:] <null> (RNTesterUnitTests:arm64+0x27c9e8)
    #8 __NSThreadPerformPerform <null> (Foundation:arm64+0x76c5e8)
    #9 __NSThread__start__ <null> (Foundation:arm64+0x76c27c)

  Previous write of size 1 at 0x000144c30be9 by thread T30:
    #0 -[RCTDeviceInfo invalidate] <null> (RNTesterUnitTests:arm64+0x434c1c)
    #1 __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ <null> (CoreFoundation:arm64+0x5e7fc)
    #2 decltype(std::declval<void () block_pointer __strong&>()()) std::__1::__invoke[abi:de180100]<void () block_pointer __strong&>(&&, decltype(std::declval<void () block_pointer __strong&>()())&&...) <null> (RNTesterUnitTests:arm64+0x2a19d4)
    #3 std::__1::__function::__func<void () block_pointer __strong, std::__1::allocator<std::__1::allocator>, void ()>::operator()() <null> (RNTesterUnitTests:arm64+0x2a16ec)
    #4 std::__1::__function::__value_func<void ()>::operator()[abi:de180100]() const <null> (RNTesterUnitTests:arm64+0x2455d4)
    #5 std::__1::function<void ()>::operator()() const <null> (RNTesterUnitTests:arm64+0x245404)
    #6 facebook::react::tryAndReturnError(std::__1::function<void ()> const&) <null> (RNTesterUnitTests:arm64+0x2c85b4)
    #7 -[RCTCxxBridge _tryAndHandleError:] <null> (RNTesterUnitTests:arm64+0x27c9e8)
    #8 __NSThreadPerformPerform <null> (Foundation:arm64+0x76c5e8)
    #9 __NSThread__start__ <null> (Foundation:arm64+0x76c27c)

  Location is heap block of size 48 at 0x000144c30bd0 allocated by main thread:
    #0 calloc <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x53b90)
    #1 _malloc_type_calloc_outlined <null> (libsystem_malloc.dylib:arm64+0xf8dc)
    #2 -[RCTModuleData setUpInstanceAndBridge:] <null> (RNTesterUnitTests:arm64+0x32715c)
    #3 __25-[RCTModuleData instance]_block_invoke <null> (RNTesterUnitTests:arm64+0x32a288)
    #4 RCTUnsafeExecuteOnMainQueueSync <null> (RNTesterUnitTests:arm64+0x3d6e1c)
    #5 -[RCTModuleData instance] <null> (RNTesterUnitTests:arm64+0x329d78)
    #6 __49-[RCTCxxBridge _prepareModulesWithDispatchGroup:]_block_invoke <null> (RNTesterUnitTests:arm64+0x287e74)
    #7 __wrap_dispatch_group_async_block_invoke <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x7cffc)
    #8 _dispatch_client_callout <null> (libdispatch.dylib:arm64+0x3c04)
    #9 __70-[XCTestCase _shouldContinueAfterPerformingSetUpSequenceWithSelector:]_block_invoke.136 <null> (XCTestCore:arm64+0x2d068)

  Thread T32 (tid=4154385, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x3027c)
    #1 -[NSThread startAndReturnError:] <null> (Foundation:arm64+0x76bec4)
    #2 -[RCTBridge setUp] <null> (RNTesterUnitTests:arm64+0x23d638)
    #3 -[RCTBridge initWithDelegate:bundleURL:moduleProvider:launchOptions:] <null> (RNTesterUnitTests:arm64+0x23addc)
    #4 -[RCTBridge initWithBundleURL:moduleProvider:launchOptions:] <null> (RNTesterUnitTests:arm64+0x23aa4c)
    #5 -[RCTImageLoaderTests testImageLoaderUsesImageDecoderWithHighestPriority] <null> (RNTesterUnitTests:arm64+0xc578)
    #6 __invoking___ <null> (CoreFoundation:arm64+0x132cdc)

  Thread T30 (tid=4154383, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_iossim_dynamic.dylib:arm64+0x3027c)
    #1 -[NSThread startAndReturnError:] <null> (Foundation:arm64+0x76bec4)
    #2 -[RCTBridge setUp] <null> (RNTesterUnitTests:arm64+0x23d638)
    #3 -[RCTBridge initWithDelegate:bundleURL:moduleProvider:launchOptions:] <null> (RNTesterUnitTests:arm64+0x23addc)
    #4 -[RCTBridge initWithBundleURL:moduleProvider:launchOptions:] <null> (RNTesterUnitTests:arm64+0x23aa4c)
    #5 -[RCTImageLoaderTests testImageDecoding] <null> (RNTesterUnitTests:arm64+0xa8d0)
    #6 __invoking___ <null> (CoreFoundation:arm64+0x132cdc)
```

The proposed solution is making the `BOOL` ivar in question a `std::atomic<BOOL>` instead.

## Changelog:

[IOS][FIXED] Data race related to read/write of RCTDeviceInfo._invalidated.

Pull Request resolved: #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
  • Loading branch information
hakonk authored and facebook-github-bot committed Jan 27, 2025
1 parent f282baa commit 2a18d83
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions packages/react-native/React/CoreModules/RCTDeviceInfo.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#import <React/RCTInvalidating.h>
#import <React/RCTUIUtils.h>
#import <React/RCTUtils.h>
#import <atomic>

#import "CoreModulesPlugins.h"

Expand All @@ -28,7 +29,7 @@ @implementation RCTDeviceInfo {
UIInterfaceOrientation _currentInterfaceOrientation;
NSDictionary *_currentInterfaceDimensions;
BOOL _isFullscreen;
BOOL _invalidated;
std::atomic<BOOL> _invalidated;
}

@synthesize moduleRegistry = _moduleRegistry;
Expand Down Expand Up @@ -96,10 +97,9 @@ - (void)initialize

- (void)invalidate
{
if (_invalidated) {
if (_invalidated.exchange(YES)) {
return;
}
_invalidated = YES;
[self _cleanupObservers];
}

Expand Down

0 comments on commit 2a18d83

Please sign in to comment.