From 243aecc095c20f8295fdefa8fc3e3ac8a7691043 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Mon, 3 Feb 2025 07:27:58 -0800 Subject: [PATCH] fix: avoid `ConcurrentModificationException` (#49109) Summary: Fixes a `ConcurrentModificationException` when iterating over `TextWatcher` `mListeners` array. If you open Android open source code (`TextView` class), then we can see that Android iterates with `for/n` loop (not `for/:`): ```java void sendAfterTextChanged(Editable text) { if (mListeners != null) { final ArrayList list = mListeners; final int count = list.size(); for (int i = 0; i < count; i++) { list.get(i).afterTextChanged(text); } } notifyListeningManagersAfterTextChanged(); hideErrorIfUnchanged(); } ```
We can catch the `ConcurrentModificationException` with old code, when for example we have 3 listeners: - 0 is `EmojiTextWatcher` (seems like it's added by OS); - 1 is `OnlyChangeIfRequiredMaskedTextChangedListener` (added by `react-native-text-input-mask`); - 2 is a listener that attached by `react-native-keyboard-controller`. On every afterTextChanged [input-mask-android](https://github.com/RedMadRobot/input-mask-android/tree/df452edc0c52a37e5082adcfc3d05d77b5aa34e8) [removes](https://github.com/RedMadRobot/input-mask-android/blob/df452edc0c52a37e5082adcfc3d05d77b5aa34e8/inputmask/src/main/kotlin/com/redmadrobot/inputmask/MaskedTextChangedListener.kt#L212) the listener and [adds](https://github.com/RedMadRobot/input-mask-android/blob/df452edc0c52a37e5082adcfc3d05d77b5aa34e8/inputmask/src/main/kotlin/com/redmadrobot/inputmask/MaskedTextChangedListener.kt#L231) it back. The oversimplified version of the code can be next: ```java public class MyClass { public static void main(String args[]) { ArrayList mListeners = new ArrayList<>(); mListeners.add(0); mListeners.add(1); mListeners.add(2); Iterator iterator = mListeners.iterator(); while (iterator.hasNext()) { Integer listener = iterator.next(); // Check if the listener is equal to 1 // 1 is OnlyChangeIfRequiredMaskedTextChangedListener and we simulate the behavior of this class if (listener == 1) { int i = mListeners.indexOf(listener); if (i >= 0) { mListeners.remove(i); } // Add the removed element at the end mListeners.add(listener); } } // Print the modified list System.out.println(mListeners); } } ``` Key points are: - if we have only [0, 1] listener, then it works well and `ConcurrentModificationException` will not be thrown, because we modify last element; - if we have `[0, 1, 2]` then exception will be thrown. So in this PR I decided to re-work code to match what Android has. With `for/n` approach `ConcurrentModificationException` will not be thrown, because we don't check array immutability in this case. More information also can be found here: https://github.com/kirillzyusko/react-native-keyboard-controller/issues/324 ## Changelog: [ANDROID] [CHANGED] - avoid `ConcurrentModificationException` when iterating over `mListeners` `TextWatcher` array Pull Request resolved: https://github.com/facebook/react-native/pull/49109 Reviewed By: cortinico Differential Revision: D69050984 Pulled By: javache fbshipit-source-id: 9c6a7a428467fa5e546d70549dfcc91d6b2e58d2 --- .../com/facebook/react/views/textinput/ReactEditText.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 2282e43bd21..42f238363e8 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -80,8 +80,8 @@ import com.facebook.react.views.text.internal.span.ReactStrikethroughSpan; import com.facebook.react.views.text.internal.span.ReactTextPaintHolderSpan; import com.facebook.react.views.text.internal.span.ReactUnderlineSpan; import com.facebook.react.views.text.internal.span.TextInlineImageSpan; -import java.util.ArrayList; import java.util.Objects; +import java.util.concurrent.CopyOnWriteArrayList; /** * A wrapper around the EditText that lets us better control what happens when an EditText gets @@ -111,7 +111,7 @@ public class ReactEditText extends AppCompatEditText { /** A count of events sent to JS or C++. */ protected int mNativeEventCount; - private @Nullable ArrayList mListeners; + private @Nullable CopyOnWriteArrayList mListeners; private @Nullable TextWatcherDelegator mTextWatcherDelegator; private int mStagedInputType; protected boolean mContainsImages; @@ -390,7 +390,7 @@ public class ReactEditText extends AppCompatEditText { @Override public void addTextChangedListener(TextWatcher watcher) { if (mListeners == null) { - mListeners = new ArrayList<>(); + mListeners = new CopyOnWriteArrayList<>(); super.addTextChangedListener(getTextWatcherDelegator()); }