From 1f4e47b00e064e606f8a0bc2111e0d8cfc79eb67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Bert?=
 <63123542+m-bert@users.noreply.github.com>
Date: Fri, 9 Feb 2024 12:56:10 +0100
Subject: [PATCH] Fix `ConcurrentModificationException`. (#2750)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Description

A while ago we decided to remove limit of gesture handlers on android (#2672). We missed one thing - `awaitngHandlers` can be modified while iterating over its items (inside [this loop](https://github.com/software-mansion/react-native-gesture-handler/blob/2e5df1c9a8595929b9879dc1246a7fd78de0549a/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt#L105)). This resulted in `ConcurrentModificationException`.

This PR changes logic so that now we iterate over copy of `awaitingHandlers`, avoiding modification of collection that we iterate through.

Even though it looks like it changes a lot, the only difference beside adding copy is changing

```jsx
for(otherHandler in currentlyAwaitingHandlers){
    if (shouldHandlerWaitForOther(otherHandler, handler)) {
        ...
    }
}
```

into

```jsx
for(otherHandler in currentlyAwaitingHandlers){
    if (!shouldHandlerWaitForOther(otherHandler, handler)) {
        continue
    }
    ...
}
```

to remove one level of nested `ifs`.

## Test plan

<details>
<summary> Modified code from #2739 </summary>

```jsx
import React from 'react';
import { ScrollView, StyleSheet, Text, View } from 'react-native';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';
import Animated from 'react-native-reanimated';

const Showcase = () => {
  const [lastActions, setLastActions] = React.useState([]);

  const addAction = (action) => {
    setLastActions((actions) => {
      const date = new Date();
      const time = date.toLocaleTimeString();

      if (actions.length >= 3) {
        actions.pop();
      }
      return [action + ': ' + time, ...actions];
    });
  };

  const longPressGesture = Gesture.LongPress()
    .onStart(() => {
      addAction('long press');
    })
    .runOnJS(true);

  const doubleTapGesture = Gesture.Tap()
    .numberOfTaps(2)
    .onStart(() => {
      addAction('double tap');
    })
    .runOnJS(true);

  const gesture = Gesture.Race(doubleTapGesture, longPressGesture);

  return (
    <View
      style={{
        flex: 1,
        backgroundColor: 'white',
        marginHorizontal: 20,
        marginVertical: 60,
      }}>
      <ScrollView contentContainerStyle={{ gap: 20 }}>
        <GestureDetector gesture={gesture}>
          <Animated.View style={styles.outer}>
            <Blue onAddAction={addAction} />
          </Animated.View>
        </GestureDetector>
        <View>
          {lastActions.map((action, index) => (
            <Text key={index}>
              {index} - {action}
            </Text>
          ))}
        </View>
        <View style={{ gap: 20 }}>
          <Text>
            <Bold>DoubleTap</Bold>: on all rectangles - should print "double
            tap" - no tap gesture should be recognized ✅
          </Text>
          <Text>
            <Bold>LongPress</Bold>: on all rectangles - should print "long
            press" - no tap gesture should be recognized ✅
          </Text>
          <Text>
            <Bold>Tap</Bold>: on red - should do nothing ✅
          </Text>
          <Text>
            <Bold>Tap</Bold>: on blue - should print "Blue: tap" ✅
          </Text>
          <Text>
            <Bold>Tap</Bold>: on orange - should print "Orange: tap" ⛔️ -
            Crashes on Android
          </Text>
        </View>
      </ScrollView>
    </View>
  );
};

export default Showcase;

const Blue = ({ onAddAction }) => {
  const tapGesture = Gesture.Tap()
    .onStart(() => {
      onAddAction('Blue: tap');
    })
    .runOnJS(true);
  const doubleTapGesture = Gesture.Tap()
    .numberOfTaps(2)
    .onStart(() => {
      onAddAction('double tap');
    })
    .runOnJS(true);

  const composedGesture = Gesture.Exclusive(doubleTapGesture, tapGesture);

  return (
    <GestureDetector gesture={composedGesture}>
      <Animated.View style={styles.blue}>
        <Orange onAddAction={onAddAction} />
      </Animated.View>
    </GestureDetector>
  );
};

const Orange = ({ onAddAction }) => {
  const tapGesture = Gesture.Tap()
    .onStart(() => {
      onAddAction('Orange: tap');
    })
    .runOnJS(true);

  const doubleTapGesture = Gesture.Tap()
    .numberOfTaps(2)
    .onStart(() => {
      onAddAction('double tap');
    })
    .runOnJS(true);

  return (
    <GestureDetector gesture={Gesture.Exclusive(doubleTapGesture, tapGesture)}>
      <Animated.View style={styles.orange}></Animated.View>
    </GestureDetector>
  );
};

const Bold = ({ children }) => {
  return <Text style={{ fontWeight: 'bold' }}>{children}</Text>;
};

const styles = StyleSheet.create({
  outer: {
    padding: 20,
    backgroundColor: 'red',
    width: 300,
    height: 200,
  },
  blue: {
    width: '80%',
    height: '80%',
    backgroundColor: 'blue',
    flexDirection: 'row',
    flexWrap: 'wrap',
  },
  orange: {
    margin: 5,
    width: '50%',
    height: '50%',
    backgroundColor: 'orange',
  },
});

```

</details>

<details>
<summary> Smaller example </summary>

```jsx
import React from 'react';
import { StyleSheet, View } from 'react-native';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';

export default function App() {
  const t3 = Gesture.Tap()
    .numberOfTaps(3)
    .onEnd(() => console.log('t3'));
  const t1_1 = Gesture.Tap().onEnd(() => console.log('t1_1'));
  const t1_2 = Gesture.Tap().onEnd(() => console.log('t1_2'));

  const g = Gesture.Exclusive(t3, t1_1, t1_2);

  return (
    <GestureDetector gesture={g}>
      <View style={styles.box} />
    </GestureDetector>
  );
}

const styles = StyleSheet.create({
  box: {
    width: 250,
    height: 250,
    backgroundColor: 'crimson',
  },
});

```
</details>
---
 .../core/GestureHandlerOrchestrator.kt        | 44 +++++++++++--------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt b/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt
index 8c9e11c10a..25c9e6f7e3 100644
--- a/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt
+++ b/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt
@@ -101,27 +101,33 @@ class GestureHandlerOrchestrator(
   fun onHandlerStateChange(handler: GestureHandler<*>, newState: Int, prevState: Int) {
     handlingChangeSemaphore += 1
     if (isFinished(newState)) {
+      // We have to loop through copy in order to avoid modifying collection
+      // while iterating over its elements
+      val currentlyAwaitingHandlers = awaitingHandlers.toList()
+
       // if there were handlers awaiting completion of this handler, we can trigger active state
-      for (otherHandler in awaitingHandlers) {
-        if (shouldHandlerWaitForOther(otherHandler, handler)) {
-          if (newState == GestureHandler.STATE_END) {
-            // gesture has ended, we need to kill the awaiting handler
-            otherHandler.cancel()
-            if (otherHandler.state == GestureHandler.STATE_END) {
-              // Handle edge case, where discrete gestures end immediately after activation thus
-              // their state is set to END and when the gesture they are waiting for activates they
-              // should be cancelled, however `cancel` was never sent as gestures were already in the END state.
-              // Send synthetic BEGAN -> CANCELLED to properly handle JS logic
-              otherHandler.dispatchStateChange(
-                GestureHandler.STATE_CANCELLED,
-                GestureHandler.STATE_BEGAN
-              )
-            }
-            otherHandler.isAwaiting = false
-          } else {
-            // gesture has failed recognition, we may try activating
-            tryActivate(otherHandler)
+      for (otherHandler in currentlyAwaitingHandlers) {
+        if (!shouldHandlerWaitForOther(otherHandler, handler)) {
+          continue
+        }
+
+        if (newState == GestureHandler.STATE_END) {
+          // gesture has ended, we need to kill the awaiting handler
+          otherHandler.cancel()
+          if (otherHandler.state == GestureHandler.STATE_END) {
+            // Handle edge case, where discrete gestures end immediately after activation thus
+            // their state is set to END and when the gesture they are waiting for activates they
+            // should be cancelled, however `cancel` was never sent as gestures were already in the END state.
+            // Send synthetic BEGAN -> CANCELLED to properly handle JS logic
+            otherHandler.dispatchStateChange(
+              GestureHandler.STATE_CANCELLED,
+              GestureHandler.STATE_BEGAN
+            )
           }
+          otherHandler.isAwaiting = false
+        } else {
+          // gesture has failed recognition, we may try activating
+          tryActivate(otherHandler)
         }
       }
       cleanupAwaitingHandlers()