software-mansion / react-native-reanimated

React Native's Animated library reimplemented
https://docs.swmansion.com/react-native-reanimated/
MIT License
8.86k stars 1.29k forks source link

[2.x] [iOS] Deadlock in `JSCObjectValue::invalidate` after migrating from `PanGestureHandler` to `Gesture.Pan` #3917

Closed terribleben closed 8 months ago

terribleben commented 1 year ago

Description

Hello - I saw some other deadlock issues here, but none of them had the same stack trace as we do, and it's unclear if they're all the same issue, so I thought this might be a new report. Sorry if this is considered a duplicate, or if it belongs on the gesture-handler repo. Hopefully some of the info here is helpful :)

We recently migrated from PanGestureHandler to a GestureDetector with Gesture.Pan(). After the migration, we started seeing deadlocks where both the UI thread and the JS thread are waiting inside JSCObjectValue::invalidate() when trying to manipulate a reanimated::ShareableValue. I've attached the full stacktrace for all threads.

Our new code is very similar to the documentation example for PanGesture. I've provided a code snippet below. I've also attached a repro repository.

As others have noted here, adding .runOnJS(true) to the gesture seems to fix the issue, but I think that means we no longer get the benefits of running on the UI thread.

Stacktrace for all threads during deadlock: reanimated-pan-gesture-deadlock.txt

Steps to reproduce

I've provided the repository terribleben/reanimated-deadlock-example. This is the smallest chunk of code I could isolate from our app that contains the same gesture. The animated component is the same as in our app, except I removed a reference to react-navigation's tab bar.

The repro steps are: Perform actions that cause the animated component to rerender or remount. Slower/expensive renders seem more likely to reproduce the issue. Due to the nature of the bug, it's not something we can get to happen with a deterministic set of steps. However, our deadlock stack trace is always the same (JSCObjectValue::invalidate).

We find it a bit easier to reproduce the issue when the Xcode address sanitizer is enabed, maybe just because it slows things down even more and makes the deadlock more likely to happen. However, the issue happens with or without this setting.

We've reproduced this issue both on a real device (iPhone XS) and also on the "my mac (designed for iPad)" Xcode target on a Macbook Air M1.

The code snippet that uses reanimated shared values looks like this:

  let snapY = useSharedValue(screenHeight);
  let gestureInitialY = useSharedValue(0);
  let isTabBarVisible = useSharedValue(false);

...

  const animatedTopYStyle = useAnimatedStyle(() => ({
    transform: [{ translateY: snapY.value }],
  }));

...

  const panGesture = Gesture.Pan()
    .onStart((e) => {
      const tabBarOffset = isTabBarVisible.value ? 65 : 0;
      gestureInitialY.value = e.absoluteY - e.y - tabBarOffset;
      snapY.value = e.translationY + gestureInitialY.value;
    })
    .onUpdate((e) => {
      snapY.value = e.translationY + gestureInitialY.value;
    })
    .onEnd((e) => {
      const { absoluteY, velocityY } = e;
      runOnJS(snapToClosest)(absoluteY, velocityY); // snapY.value = withSpring(...)
    });

...

return (
    <Animated.View style={[styles.container, style, animatedTopYStyle]}>
      <GestureDetector gesture={panGesture}>
         <Animated.View>{renderHeader()}</Animated.View>
      </GestureDetector>

Snack or a link to a repository

https://github.com/terribleben/reanimated-deadlock-example

Reanimated version

2.12.0

React Native version

0.69.4

Platforms

iOS

JavaScript runtime

JSC

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

Debug mode

Device

Real device

Device model

iPhone XS running iOS 15.1

Acknowledgements

Yes

tomekzaw commented 1 year ago

Hey @terribleben, thanks for reporting this issue.

Can you please check if react-native-reanimated@3.0.0-rc.9 resolves this issue? Reanimated 3 comes with a new implementation of shared values that, among many changes, removes reanimated::ShareableValue.

terribleben commented 1 year ago

Hey @tomekzaw, thanks for getting back to me.

I'll try it with the reanimated 3 release candidate in the smaller repo and see what happens - thanks for the suggestion. However, it might be too risky for us to deploy a release candidate in our main production app, so I'm also interested to see if there's a fix that can be applied to Reanimated 2. We are also hitting some other stability issues (see other issue comment in this thread) so I'm concerned we might need to revert back to PanGestureHandler.

mhammerc commented 1 year ago

Hello! Just to say we have the exact same bug.

Our app hang on JSCObjectValue::invalidate() with reanimated::ShareableValue::toJSValue(facebook::jsi::Runtime&) in the trace (when pausing using xcode debugger).

Upgrading to react-native-reanimated@3.0.0-rc.10 seems to fix the hang.

terribleben commented 1 year ago

Sure enough, I tried bumping the repro repository to the most recent reanimated 3, and I haven't seen the deadlock issue yet (which is about the best one could say).

I don't have a good sense of how stable reanimated 3 prerelease is, so I'm unsure whether to try it in our production app which has a pretty large userbase across a lot of different devices. All of the official Reanimated docs still refer to v2 as the stable version as far as I can tell. Am I correct to guess that software mansion is mostly focused on reanimated 3 right now, and not as much on reanimated 2 stability?

Thanks again :)

tomekzaw commented 1 year ago

I haven't seen the deadlock issue yet (which is about the best one could say).

Oh, that's great news!

I don't have a good sense of how stable reanimated 3 prerelease is Am I correct to guess that software mansion is mostly focused on reanimated 3 right now, and not as much on reanimated 2 stability?

That's correct, v3 is released directly from main whereas only some critical PRs (like 0.71 support) are backported to 2.x branch. Also, Reanimated 3 comes with a new implementation of shared values which is ~2x faster and also eliminates some random crashes that occurred on 2.x and haven't been fixed yet. Obviously, it is not yet as battle-tested as 2.x but if it works for you, you should be fine. In fact, we plan one or two more rc releases and then we'll publish 3.0.0 (stable).

terribleben commented 1 year ago

Thanks for the information, that will help us decide when to switch over.

terribleben commented 1 year ago

Hi, just an update that this particular issue is not fixed by the 2.15+ update that includes this other ANR fix (we tried 2.17) https://github.com/software-mansion/react-native-reanimated/pull/4283