Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RN 0.66 Switch causes a stuck operation in mViewCommandOperations #32594

Closed
jonathanmos opened this issue Nov 15, 2021 · 1 comment
Closed

RN 0.66 Switch causes a stuck operation in mViewCommandOperations #32594

jonathanmos opened this issue Nov 15, 2021 · 1 comment
Labels
Component: Switch Needs: Triage 🔍 Resolution: Locked This issue was locked by the bot. Type: Upgrade Issue Issues reported from upgrade issue form

Comments

@jonathanmos
Copy link
Contributor

New Version

0.66.1

Old Version

0.65.2

Build Target(s)

Android Release

Output of react-native info

System:
OS: macOS 12.0.1
CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
Memory: 497.72 MB / 16.00 GB
Shell: 5.8 - /bin/zsh
Binaries:
Node: 12.5.0 - /usr/local/opt/nvm/versions/node/v12.5.0/bin/node
Yarn: 1.22.17 - /usr/local/opt/nvm/versions/node/v12.5.0/bin/yarn
npm: 6.9.0 - /usr/local/opt/nvm/versions/node/v12.5.0/bin/npm
Watchman: 2021.06.07.00 - /usr/local/bin/watchman
Managers:
CocoaPods: 1.10.1 - /usr/local/bin/pod
SDKs:
iOS SDK:
Platforms: iOS 14.4, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
Android SDK:
API Levels: 28, 29, 30, 31
Build Tools: 29.0.2, 29.0.3, 30.0.0, 30.0.2, 31.0.0
System Images: android-21 | Intel x86 Atom_64, android-22 | Intel x86 Atom_64, android-22 | Google APIs Intel x86 Atom, android-23 | Intel x86 Atom_64, android-26 | Google APIs Intel x86 Atom_64, android-28 | Intel x86 Atom_64, android-28 | Google APIs Intel x86 Atom_64, android-29 | Intel x86 Atom_64, android-29 | Google Play Intel x86 Atom_64, android-30 | Google APIs Intel x86 Atom, android-30 | Google Play Intel x86 Atom, android-31 | Google APIs ARM 64 v8a, android-31 | Google Play ARM 64 v8a, android-31 | Google Play Intel x86 Atom_64
Android NDK: 23.0.7344513-beta4
IDEs:
Android Studio: 4.1 AI-201.8743.12.41.7199119
Xcode: 12.4/12D4e - /usr/bin/xcodebuild
Languages:
Java: 1.8.0_292 - /usr/bin/javac
npmPackages:
@react-native-community/cli: Not Found
react: 17.0.2 => 17.0.2
react-native: 0.66.1 => 0.65.2
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found

Issue and Reproduction Steps

In UIViewOperationQueue there is a list of ViewOperations called mViewCommandOperations. Detox checks that this list is empty, among other things, in order to determine that a device has reached an idle state.

In RN 0.66 a regression occured. In Android Release, the Switch component attempts to perform setNativeValue true/false during initial layout on the screen, and it very often fails with a RetryableMountingLayerException of "Trying to send command to a non-existing view with tag..", it then increments the operation retry counter, but the operation remains stuck in mViewCommandOperations and is not cleared until some further action occurs on the screen (for example, clicking on the switch). This leads to a situation where, as far as Detox is concerned, the device is not idle and our tests begin to timeout.

Here is the relevant section in the code:

// Catch errors in DispatchCommands. We allow all commands to be retried

I've managed to locate the commit where this regression started to occur, apparently connected to refactoring the switch to a functional component: ab66741

There are two things that are not clear here:

  • Why is a RetryableMountingLayerException occurring so often on initial layout of the Switch?
  • Why is mViewCommandOperations not performing the retry and clearing the queue immediately, without awaiting further user interaction?

To reproduce this issue:
Go into a screen containing a Switch component in Android Release. Poll the isEmpty method of UIViewOperationQueue and you will see that it returns false.

@jonathanmos
Copy link
Contributor Author

jonathanmos commented Nov 15, 2021

The RetryableMountingLayerException is apparently being caused by the use of useLayoutEffect instead of useEffect in Switch.js. Changing to useEffect solves that problem - I will open a PR to change it.

ab66741?diff=unified#diff-4b690e8beb33507f35317e5ed056151a3bcec03781b5c731e6317cb479e164d5R158

jonathanmos added a commit to jonathanmos/react-native that referenced this issue Nov 16, 2021
Summary:
Changelog:
Change useLayoutEffect to useEffect to fix facebook#32594 (stuck operation in viewcommands list in Android Release)
jonathanmos added a commit to jonathanmos/react-native that referenced this issue Nov 16, 2021
Summary:
Changelog:
Change useLayoutEffect to useEffect to fix facebook#32594 - stuck operation in mViewCommandOperations list in Android Release
ShikaSD pushed a commit that referenced this issue Mar 16, 2022
Summary:
Added a null check to native.value in Switch to fix regression from RN 66 -> stuck operation in mViewCommandOperations list in Android Release on initial layout of a screen with Switch component. If approved, please incorporate this fix into an RN 66 release.

## Changelog
[Android][Fixed] - Added a null check to native.value in Switch to fix #32594

Pull Request resolved: #32602

Test Plan: To reproduce, put a log in UIViewOperationQueue in dispatchViewUpdates you can see that the RetryableMountingException is no longer thrown for a screen with the Switch component on Android Release. As a result, mViewCommandOperations no longer has a stuck operation on initial layout.

Reviewed By: charlesbdudley

Differential Revision: D34397788

Pulled By: lunaleaps

fbshipit-source-id: 1cee3516fb987942dfa50ad1f2d11c965a947f05
ShikaSD pushed a commit that referenced this issue Mar 18, 2022
Summary:
Added a null check to native.value in Switch to fix regression from RN 66 -> stuck operation in mViewCommandOperations list in Android Release on initial layout of a screen with Switch component. If approved, please incorporate this fix into an RN 66 release.

## Changelog
[Android][Fixed] - Added a null check to native.value in Switch to fix #32594

Pull Request resolved: #32602

Test Plan: To reproduce, put a log in UIViewOperationQueue in dispatchViewUpdates you can see that the RetryableMountingException is no longer thrown for a screen with the Switch component on Android Release. As a result, mViewCommandOperations no longer has a stuck operation on initial layout.

Reviewed By: charlesbdudley

Differential Revision: D34397788

Pulled By: lunaleaps

fbshipit-source-id: 1cee3516fb987942dfa50ad1f2d11c965a947f05
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this issue Jan 15, 2023
Summary:
Added a null check to native.value in Switch to fix regression from RN 66 -> stuck operation in mViewCommandOperations list in Android Release on initial layout of a screen with Switch component. If approved, please incorporate this fix into an RN 66 release.

## Changelog
[Android][Fixed] - Added a null check to native.value in Switch to fix facebook#32594

Pull Request resolved: facebook#32602

Test Plan: To reproduce, put a log in UIViewOperationQueue in dispatchViewUpdates you can see that the RetryableMountingException is no longer thrown for a screen with the Switch component on Android Release. As a result, mViewCommandOperations no longer has a stuck operation on initial layout.

Reviewed By: charlesbdudley

Differential Revision: D34397788

Pulled By: lunaleaps

fbshipit-source-id: 1cee3516fb987942dfa50ad1f2d11c965a947f05
@facebook facebook locked as resolved and limited conversation to collaborators Feb 25, 2023
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: Switch Needs: Triage 🔍 Resolution: Locked This issue was locked by the bot. Type: Upgrade Issue Issues reported from upgrade issue form
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants