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

Crash when navigating to 2 screens at the same time on new arch #2235

Closed
janicduplessis opened this issue Jul 5, 2024 · 15 comments · Fixed by #2249
Closed

Crash when navigating to 2 screens at the same time on new arch #2235

janicduplessis opened this issue Jul 5, 2024 · 15 comments · Fixed by #2249
Assignees
Labels
NewArch Issues related only to new architecture Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@janicduplessis
Copy link
Contributor

Description

When navigating to 2 screens at the same time on new arch it crashes with the following error:

navigation.navigate('a');
navigation.navigate('b');
"<RNSNavigationController> is pushing the same view controller instance (<RNSScreen>) more than once which is not supported and is most likely an error in the application"

Steps to reproduce

Using this code in FabricExample app SimpleNativeStack.tsx

  • Go to "Simple Native Stack" example
  • Click on "Go to detail"
  • App crashes

Snack or a link to a repository

https://github.com/janicduplessis/react-native-screens/tree/%40janic/double-push

Screens version

main@e5a6220

React Native version

0.74

Platforms

iOS

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Fabric (New Architecture)

Build type

Debug mode

Device

iOS simulator

Device model

No response

Acknowledgements

Yes

@github-actions github-actions bot added Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided labels Jul 5, 2024
@kkafar kkafar added the NewArch Issues related only to new architecture label Jul 8, 2024
@tboba
Copy link
Member

tboba commented Jul 8, 2024

Hi @janicduplessis, thanks for submitting this issue! I remember I've tried to fix this issue couple months ago (here, but this change has ended in revealing another issues with view traversing 😕 I'll try to find out what's going on here, thanks!

@tboba tboba self-assigned this Jul 8, 2024
@RalissonMattias
Copy link

@tboba Is this problem similar to this one software-mansion/react-native-reanimated#6222 ? Otherwise I can open a new issue

@tboba
Copy link
Member

tboba commented Jul 9, 2024

@RalissonMattias can't say if this issue is related to screens, as there's too few info attached in the original issue. Could you post us a full log trace from the error? Also, are there any errors printed in the JS console?

@RalissonMattias
Copy link

@RalissonMattias can't say if this issue is related to screens, as there's too few info attached in the original issue. Could you post us a full log trace from the error? Also, are there any errors printed in the JS console?

Here is more infos

Screenshot 2024-07-09 at 10 50 39

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Attempt to unmount a view which is mounted inside different view. (parent: <RNSScreenView: 0x115537200; frame = (0 0; 393 710); tag = 150; layer = <CALayer: 0x600000308280>>, child: <RCTViewComponentView: 0x117756440; frame = (0 0; 393 44); tag = 144; layer = <CALayer: 0x60000030c840>>, index: 3)'
*** First throw call stack:
(
	0   CoreFoundation                      0x0000000180491128 __exceptionPreprocess + 172
	1   libobjc.A.dylib                     0x000000018008412c objc_exception_throw + 56
	2   Foundation                          0x0000000180d11770 -[NSMutableDictionary(NSMutableDictionary) classForCoder] + 0
	3   reanimatedWithBottomTabsTest        0x0000000104eb8f34 -[RCTViewComponentView unmountChildComponentView:index:] + 560
	4   reanimatedWithBottomTabsTest        0x000000010488d614 -[RNSScreenView unmountChildComponentView:index:] + 244
	5   reanimatedWithBottomTabsTest        0x0000000104e10f3c _ZL27RCTPerformMountInstructionsRKNSt3__16vectorIN8facebook5react18ShadowViewMutationENS_9allocatorIS3_EEEEP24RCTComponentViewRegistryR41RCTMountingTransactionObserverCoordinatori + 1344
	6   reanimatedWithBottomTabsTest        0x0000000104e109f0 _ZZ41-[RCTMountingManager performTransaction:]ENK3$_1clERKN8facebook5react19MountingTransactionERKNS1_16SurfaceTelemetryE + 80
	7   reanimatedWithBottomTabsTest        0x0000000104e10994 _ZNSt3__18__invokeB7v160006IRZ41-[RCTMountingManager performTransaction:]E3$_1JRKN8facebook5react19MountingTransactionERKNS4_16SurfaceTelemetryEEEEDTclclsr3stdE7declvalIT_EEspclsr3stdE7declvalIT0	8   reanimatedWithBottomTabsTest        0x0000000104e1093c _ZNSt3__128__invoke_void_return_wrapperIvLb1EE6__callIJRZ41-[RCTMountingManager performTransaction:]E3$_1RKN8facebook5react19MountingTransactionERKNS6_16SurfaceTelemetryEEEEvDpOT_ + 40
	9   reanimatedWithBottomTabsTest        0x0000000104e10908 _ZNSt3__110__function12__alloc_funcIZ41-[RCTMountingManager performTransaction:]E3$_1NS_9allocatorIS2_EEFvRKN8facebook5react19MountingTransactionERKNS6_16SurfaceTelemetryEEEclB7v160006ES9_SC_ + 4	10  reanimatedWithBottomTabsTest        0x0000000104e0f774 _ZNSt3__110__function6__funcIZ41-[RCTMountingManager performTransaction:]E3$_1NS_9allocatorIS2_EEFvRKN8facebook5react19MountingTransactionERKNS6_16SurfaceTelemetryEEEclES9_SC_ + 44
	11  reanimatedWithBottomTabsTest        0x0000000104c462ac _ZNKSt3__110__function12__value_funcIFvRKN8facebook5react19MountingTransactionERKNS3_16SurfaceTelemetryEEEclB7v160006ES6_S9_ + 84
	12  reanimatedWithBottomTabsTest        0x0000000104c45364 _ZNKSt3__18functionIFvRKN8facebook5react19MountingTransactionERKNS2_16SurfaceTelemetryEEEclES5_S8_ + 40
	13  reanimatedWithBottomTabsTest        0x0000000104c451c0 _ZNK8facebook5react19TelemetryController15pullTransactionERKNSt3__18functionIFvRKNS0_19MountingTransactionERKNS0_16SurfaceTelemetryEEEESD_SD_ + 292
	14  reanimatedWithBottomTabsTest        0x0000000104e0ad38 -[RCTMountingManager performTransaction:] + 480
	15  reanimatedWithBottomTabsTest        0x0000000104e0ab28 -[RCTMountingManager initiateTransaction:] + 372
	16  reanimatedWithBottomTabsTest        0x0000000104e0a53c __42-[RCTMountingManager scheduleTransaction:]_block_invoke + 324
	17  reanimatedWithBottomTabsTest        0x0000000104a513d4 __RCTExecuteOnMainQueue_block_invoke + 40
	18  libdispatch.dylib                   0x000000010876c0f0 _dispatch_call_block_and_release + 24
	19  libdispatch.dylib                   0x000000010876d93c _dispatch_client_callout + 16
	20  libdispatch.dylib                   0x000000010877d5e4 _dispatch_main_queue_drain + 1228
	21  libdispatch.dylib                   0x000000010877d108 _dispatch_main_queue_callback_4CF + 40
	22  CoreFoundation                      0x00000001803f1a30 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 12
	23  CoreFoundation                      0x00000001803ec148 __CFRunLoopRun + 1936
	24  CoreFoundation                      0x00000001803eb5a4 CFRunLoopRunSpecific + 572
	25  GraphicsServices                    0x000000018e9fbae4 GSEventRunModal + 160
	26  UIKitCore                           0x00000001852f02e4 -[UIApplication _run] + 868
	27  UIKitCore                           0x00000001852f3f5c UIApplicationMain + 124
	28  reanimatedWithBottomTabsTest        0x00000001046750cc main + 96
	29  dyld                                0x0000000107d89544 start_sim + 20
	30  ???                                 0x000000010789e0e0 0x0 + 4421443808
	31  ???                                 0xbd42800000000000 0x0 + 13637603359119638528
)
libc++abi: terminating due to uncaught exception of type NSException

@tboba
Copy link
Member

tboba commented Jul 9, 2024

@RalissonMattias that's strange indeed. Are you trying to do something similar as in this issue (navigate to two different screens)? If not, I think you can post an issue with the reproducer, so we could check this error on our side.

@RalissonMattias
Copy link

RalissonMattias commented Jul 9, 2024

@tboba Yes, I'm trying to navigate to two different screens, but using tab navigation. A curious fact is that when I use react-native-reanimated in version 3.12.1 the application does not break, however when I use react-native-reanimated in version 3.13.0 the application breaks. That's why I initially posted the issue on react-native-reanimated

@Titozzz
Copy link
Contributor

Titozzz commented Jul 9, 2024

https://github.com/Titozzz/crash-screens-tabs-new-arch
@RalissonMattias this is a repro for your bug (and mine)
It actually related to Reanimated 🤯

@janicduplessis
Copy link
Contributor Author

I actually just hit that bug too, I can confirm it is a regression in reanimated v3.13.

@janicduplessis
Copy link
Contributor Author

I do not think they are related tho

@dppo
Copy link

dppo commented Jul 10, 2024

The same problem in react-native-reanimated v3.13, and it became normal after use v3.12.1.

@tboba
Copy link
Member

tboba commented Jul 11, 2024

Looks like software-mansion/react-native-reanimated#6222 has been reopened, so for further discussions about the bug with bottom-tabs, please head over there.

@tboba
Copy link
Member

tboba commented Jul 11, 2024

I can confirm this issue is reproducible 🫡

@tboba
Copy link
Member

tboba commented Jul 16, 2024

Hey hey @janicduplessis, could you try to get screens from branch @tboba/fix-screen-traversing and check if everything works fine on your side? 😄

@janicduplessis
Copy link
Contributor Author

@tboba Works fine now, thanks!

@kkafar
Copy link
Member

kkafar commented Jul 22, 2024

I wonder whether we should pursue reporting this to RN team & trying to fix this upstream. Since there is a discrepancy between Paper & Fabric updates batching I believe this could be considered as an unintended change, similarly to this one.

@kkafar kkafar closed this as completed in 68dabd9 Jul 26, 2024
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
…oftware-mansion#2249)

## Description

On Paper, while user tries to navigate to 2 or more screens at the same
time, updates were batched (on async thread), resulting rerendering
stack hierarchy on the next layout. In the same scenario, on Fabric,
updates are not being batched, which causes crash:

```
"<RNSNavigationController> is pushing the same view controller instance (<RNSScreen>) more than once which is not supported and is most likely an error in the application"
```

since `maybeAddToParentAndUpdateContainer` is being called to early.
This PR fixes this bug by moving the logic of calling that method on
`mountingTransactionDidMount` method.

I've also checked if this change does not call
`maybeAddToParentAndUpdateContainer` too frequent and I can't see more
than one correct call there (every doubled call is being catched by
`return` in conditions).

Fixes software-mansion#2235.

## Changes

- Moved updating containers to `mountingTransactionDidMount` method.

## Screenshots / GIFs

### Before


https://github.com/user-attachments/assets/24daf575-d06c-4972-9166-89454be60126

### After


https://github.com/user-attachments/assets/d377c527-0b2c-4850-b0b5-8e93dd5f25ac

## Test code and steps to reproduce

You can use `Test2235.tsx` to test this change.

## Checklist

- [X] Included code example that can be used to test this change
- [x] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NewArch Issues related only to new architecture Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided
Projects
None yet
6 participants