From a2913d33a66dc1d179ced170d6c73c8ddf91960e Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sat, 1 Jun 2019 20:04:27 -0700 Subject: [PATCH] Fabric: The second attempt to fix a thread-safety issue in RCTNativeAnimatedModule Summary: Previously we tried to fix that with RCTUnsafeExecuteOnUIManagerQueueSync but that caused a deadlock (yeah, it's actually unsafe). Besides that, I tried to solve that with introducing a mutex that covers access to `_operations` and `_preOperations` but failed miserably. It solved threading issue but that didn't fix data-races and inconsistency of the collections. Reviewed By: sahrens Differential Revision: D15587564 fbshipit-source-id: d1953036b09354d1663a9b191440f8b4a4e6be9d --- .../NativeAnimation/RCTNativeAnimatedModule.m | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedModule.m b/Libraries/NativeAnimation/RCTNativeAnimatedModule.m index 69bf078f804..0bbc9c78711 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedModule.m +++ b/Libraries/NativeAnimation/RCTNativeAnimatedModule.m @@ -232,27 +232,31 @@ RCT_EXPORT_METHOD(removeAnimatedEventFromView:(nonnull NSNumber *)viewTag - (void)willMountComponentsWithRootTag:(NSInteger)rootTag { RCTAssertMainQueue(); - __block NSArray *preOperations; - RCTUnsafeExecuteOnUIManagerQueueSync(^{ - preOperations = self->_preOperations; + RCTExecuteOnUIManagerQueue(^{ + NSArray *preOperations = self->_preOperations; self->_preOperations = [NSMutableArray new]; + + RCTExecuteOnMainQueue(^{ + for (AnimatedOperation preOperation in preOperations) { + preOperation(self->_nodesManager); + } + }); }); - for (AnimatedOperation operation in preOperations) { - operation(self->_nodesManager); - } } - (void)didMountComponentsWithRootTag:(NSInteger)rootTag { RCTAssertMainQueue(); - __block NSArray *operations; - RCTUnsafeExecuteOnUIManagerQueueSync(^{ - operations = self->_operations; + RCTExecuteOnUIManagerQueue(^{ + NSArray *operations = self->_operations; self->_operations = [NSMutableArray new]; + + RCTExecuteOnMainQueue(^{ + for (AnimatedOperation operation in operations) { + operation(self->_nodesManager); + } + }); }); - for (AnimatedOperation operation in operations) { - operation(self->_nodesManager); - } } #pragma mark - RCTUIManagerObserver