From d3c5a7f3a8512d698474f95bc49800bb548ce63a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Knutzen?= <2263015+hakonk@users.noreply.github.com> Date: Wed, 26 Jun 2024 19:46:50 +0200 Subject: [PATCH 1/3] Fix data race related to reading/writing to AllocationTestModule.valid(). This entails making AllocationTestModule and Objective-C++ file in order to make use of std::atomic. Getters and setters are then implemented to make sure and underlying atomic bool is accessed instead of a synthesized property. --- ...TAllocationTests.m => RCTAllocationTests.mm} | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) rename packages/rn-tester/RNTesterUnitTests/{RCTAllocationTests.m => RCTAllocationTests.mm} (96%) diff --git a/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.m b/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm similarity index 96% rename from packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.m rename to packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm index ea66d188978f..11092b23d308 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.m +++ b/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm @@ -13,6 +13,7 @@ #import #import #import +#import @interface AllocationTestModule : NSObject @@ -20,7 +21,17 @@ @interface AllocationTestModule : NSObject @end -@implementation AllocationTestModule +@implementation AllocationTestModule { + std::atomic _valid; +} + +-(BOOL)isValid { + return _valid; +} + +-(void)setValid:(BOOL)newValue { + _valid = newValue; +} RCT_EXPORT_MODULE(); @@ -114,6 +125,7 @@ - (void)testModulesAreInvalidated launchOptions:nil]; XCTAssertTrue(module.isValid, @"AllocationTestModule should be valid"); (void)bridge; + [bridge invalidate]; } RCT_RUN_RUNLOOP_WHILE(module.isValid) @@ -132,7 +144,9 @@ - (void)testModulesAreDeallocated launchOptions:nil]; XCTAssertNotNil(module, @"AllocationTestModule should have been created"); weakModule = module; + [bridge invalidate]; (void)bridge; + } RCT_RUN_RUNLOOP_WHILE(weakModule) @@ -167,6 +181,7 @@ - (void)testContentViewIsInvalidated RCT_RUN_RUNLOOP_WHILE(!(rootContentView = [rootView valueForKey:@"contentView"])) XCTAssertTrue(rootContentView.userInteractionEnabled, @"RCTContentView should be valid"); (void)rootView; + [bridge invalidate]; } #if !TARGET_OS_TV // userInteractionEnabled is true for Apple TV views From 18ec4972c9e8442097d1e75536d8a03ff4ba27d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Knutzen?= <2263015+hakonk@users.noreply.github.com> Date: Wed, 26 Jun 2024 19:53:26 +0200 Subject: [PATCH 2/3] Revert unintended changes --- packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm b/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm index 11092b23d308..a49281da22ba 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm +++ b/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm @@ -125,7 +125,6 @@ - (void)testModulesAreInvalidated launchOptions:nil]; XCTAssertTrue(module.isValid, @"AllocationTestModule should be valid"); (void)bridge; - [bridge invalidate]; } RCT_RUN_RUNLOOP_WHILE(module.isValid) @@ -144,9 +143,7 @@ - (void)testModulesAreDeallocated launchOptions:nil]; XCTAssertNotNil(module, @"AllocationTestModule should have been created"); weakModule = module; - [bridge invalidate]; (void)bridge; - } RCT_RUN_RUNLOOP_WHILE(weakModule) @@ -181,7 +178,6 @@ - (void)testContentViewIsInvalidated RCT_RUN_RUNLOOP_WHILE(!(rootContentView = [rootView valueForKey:@"contentView"])) XCTAssertTrue(rootContentView.userInteractionEnabled, @"RCTContentView should be valid"); (void)rootView; - [bridge invalidate]; } #if !TARGET_OS_TV // userInteractionEnabled is true for Apple TV views From 1d31106072b8dd6d92d520476b1b2aa4b8857b7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Knutzen?= <2263015+hakonk@users.noreply.github.com> Date: Fri, 28 Jun 2024 11:14:40 +0200 Subject: [PATCH 3/3] Make valid an objc atomic property + rename to objc file --- ...llocationTests.mm => RCTAllocationTests.m} | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) rename packages/rn-tester/RNTesterUnitTests/{RCTAllocationTests.mm => RCTAllocationTests.m} (96%) diff --git a/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm b/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.m similarity index 96% rename from packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm rename to packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.m index a49281da22ba..ed1f3a7e5da1 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.mm +++ b/packages/rn-tester/RNTesterUnitTests/RCTAllocationTests.m @@ -13,39 +13,28 @@ #import #import #import -#import @interface AllocationTestModule : NSObject -@property (nonatomic, assign, getter=isValid) BOOL valid; +@property (atomic, assign, getter=isValid) BOOL valid; @end -@implementation AllocationTestModule { - std::atomic _valid; -} - --(BOOL)isValid { - return _valid; -} - --(void)setValid:(BOOL)newValue { - _valid = newValue; -} +@implementation AllocationTestModule RCT_EXPORT_MODULE(); - (instancetype)init { if ((self = [super init])) { - _valid = YES; + self.valid = YES; } return self; } - (void)invalidate { - _valid = NO; + self.valid = NO; } RCT_EXPORT_METHOD(test