Fix Unable to find module for UIManager issue

[kevinvangelder]

Summary

A race condition sometimes causes redbox errors when reloading the app after previously opening the Chrome debugger. In the project I was working on, we were regularly getting Unable to find module for UIManager and I was able to track it down to _moduleDataByName sometimes being empty or nil. This change safely early-exits when that is the case and the app loads successfully. Related issues:
#24549
#23235

Changelog

[iOS] [Fixed] - Race condition causing redbox errors when reloading app after using Chrome debugger

Test Plan

As this is a difficult-to-reproduce race condition, I'm not entirely certain what the test plan should be. I created this change as a patch to React Native in the closed-source project I've been working on and it works flawlessly, however I haven't been able to reproduce the issue in a clean project. This is a fairly safe change as the early exit only ever occurs if the variable is empty and it seems to be resilient enough to load the module again after the race condition has been defeated, but I'm open to suggestions on potential options to test this.

[mmmulani]

rather than change the module loading logic, I would change uiManager's getter here: https://github.com/facebook/react-native/blob/master/React/Modules/RCTUIManager.m#L1616

that seems to make more sense as it looks like the dealloc is throwing the error

[kevinvangelder]

Is having an early exit when _moduleDataByName is nil a bad thing? We can certainly also address other weaknesses, but I doubt there is any situation where we would want to receive a redbox error instead of having it gracefully handled.

[mmmulani]

Is having an early exit when _moduleDataByName is nil a bad thing?

yes. you're essentially checking to see if RCTUIManager has been invalidated and if so skip some checks. But we can instead harden when the RCTUIManager is accessed off RCTCxxBridge

We can certainly also address other weaknesses, but I doubt there is any situation where we would want to receive a redbox error instead of having it gracefully handled.

yea I definitely agree that we do not want to show a redbox in this case but I think that we should solve the problem a different way.

[kevinvangelder]

@mmmulani Okay. Unfortunately I have no C++ experience, so I'm not sure what the alternative solution would look like. If it's a simple change would you be willing to open a PR and I'll close this one in favor of it?

[spacesuitdiver]

Just helping document the issue, on the project I use it happens every time in the scenario @kevinvangelder described.

[kevinvangelder]

I'm curious if @mhorowitz has any thoughts for us.

[satbirkira]

Any update on this?

[github-actions]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This PR was closed because it has been stalled for 7 days with no activity.