Skip to content

Commit

Permalink
Fix issue causing ANR when performing sync layout updates, for Reanim…
Browse files Browse the repository at this point in the history
…ated 2 (#4283)

## Summary

The port of
#4239 PR
for Reanimated 2.
  • Loading branch information
piaskowyk authored Mar 27, 2023
1 parent f7e0bd5 commit 2905e1b
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-v8.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
run: npx react-native init app --version ${{ env.REACT_NATIVE_VERSION }}
- name: Install dependencies
working-directory: app
run: yarn add github:software-mansion/react-native-reanimated#${{ github.ref }} react-native-v8 v8-android-jit
run: yarn add github:software-mansion/react-native-reanimated#${{ github.ref }} react-native-v8@1.6.0 v8-android-jit@10.100.1
- name: Configure V8
run: node reanimated_repo/.github/workflows/helper/configureV8.js
- name: Build Android app
Expand Down
13 changes: 6 additions & 7 deletions android/src/main/java/com/swmansion/reanimated/NodesManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -248,13 +249,11 @@ public void runGuarded() {
}
});
if (trySynchronously) {
while (true) {
try {
semaphore.acquire();
break;
} catch (InterruptedException e) {
// noop
}
try {
semaphore.tryAcquire(16, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
// if the thread is interruped we just continue and let the layout update happen
// asynchronously
}
}
}
Expand Down
27 changes: 23 additions & 4 deletions ios/REANodesManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ @implementation REANodesManager {
BOOL _tryRunBatchUpdatesSynchronously;
REAEventHandler _eventHandler;
volatile void (^_mounting)(void);
NSObject *_syncLayoutUpdatesWaitLock;
volatile BOOL _syncLayoutUpdatesWaitTimedOut;
NSMutableDictionary<NSNumber *, ComponentUpdate *> *_componentUpdateBuffer;
volatile atomic_bool _shouldFlushUpdateBuffer;
NSMutableDictionary<NSNumber *, UIView *> *_viewRegistry;
Expand All @@ -128,6 +130,7 @@ - (instancetype)initWithModule:(REAModule *)reanimatedModule uiManager:(RCTUIMan
_operationsInBatch = [NSMutableArray new];
_componentUpdateBuffer = [NSMutableDictionary new];
_viewRegistry = [_uiManager valueForKey:@"_viewRegistry"];
_syncLayoutUpdatesWaitLock = [NSObject new];
_shouldFlushUpdateBuffer = false;
}

Expand Down Expand Up @@ -232,8 +235,14 @@ - (void)onAnimationFrame:(CADisplayLink *)displayLink
- (BOOL)uiManager:(RCTUIManager *)manager performMountingWithBlock:(RCTUIManagerMountingBlock)block
{
RCTAssert(_mounting == nil, @"Mouting block is expected to not be set");
_mounting = block;
return YES;
@synchronized(_syncLayoutUpdatesWaitLock) {
if (_syncLayoutUpdatesWaitTimedOut) {
return NO;
} else {
_mounting = block;
return YES;
}
}
}

- (void)performOperations
Expand All @@ -250,6 +259,7 @@ - (void)performOperations

__weak typeof(self) weakSelf = self;
dispatch_semaphore_t semaphore = dispatch_semaphore_create(0);
_syncLayoutUpdatesWaitTimedOut = NO;
RCTExecuteOnUIManagerQueue(^{
__typeof__(self) strongSelf = weakSelf;
if (strongSelf == nil) {
Expand All @@ -266,15 +276,24 @@ - (void)performOperations
}

if (canUpdateSynchronously) {
[strongSelf.uiManager runSyncUIUpdatesWithObserver:self];
[strongSelf.uiManager runSyncUIUpdatesWithObserver:strongSelf];
dispatch_semaphore_signal(semaphore);
}
// In case canUpdateSynchronously=true we still have to send uiManagerWillPerformMounting event
// to observers because some components (e.g. TextInput) update their UIViews only on that event.
[strongSelf.uiManager setNeedsLayout];
});
if (trySynchronously) {
dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER);
// The 16ms timeout here aims to match the frame duration. It may make sense to read that parameter
// from CADisplayLink but it is easier to hardcode it for the time being.
// The reason why we use frame duration here is that if takes longer than one frame to complete layout tasks
// there is no point of synchronizing layout with the UI interaction as we get that one frame delay anyways.
long result = dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, 16 * NSEC_PER_MSEC));
if (result != 0) {
@synchronized(_syncLayoutUpdatesWaitLock) {
_syncLayoutUpdatesWaitTimedOut = YES;
}
}
}

if (_mounting) {
Expand Down

0 comments on commit 2905e1b

Please sign in to comment.