Skip to content

fix: schedule delayed freeze with setTimeout to prevent React infinite loop #2963

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

Merged
merged 4 commits into from
Jun 9, 2025

Conversation

kligarski
Copy link
Contributor

@kligarski kligarski commented Jun 5, 2025

Description

When analyzing Test791 in order to create an e2e test for it, I noticed some weird behavior that is probably caused by multiple overlapping issues. This PR addresses one of them - when pushing modals with a header in setInterval from a regular push screen that is not freezed (we prevent freezing it by using freezeOnBlur: false option), only 4 of them are pushed. Then the interval stops and the screens are not being pushed anymore. What is even more interesting, interacting with a button in the modal fixes the issue, sometimes only for one more push, but sometimes it just starts working without any issues.

DelayedFreeze component was introduced in #1220 in order to fix react-navigation's useIsFocused hook and some animation bugs. DelayedFreeze allows one more render before freezing the screen. If we change freeze to true, we still pass previous freezeState (false) to the Freeze component. useEffect runs and uses setImmediate to update the freezeState right after the render. This mechanism works for regular push/card screens but for some unknown reason, it does not work with multiple screens with presentation: 'modal' and headerShown: true (more details below). Changing setImmediate to setInterval(fn, 0) fixes the issue.

EDIT: Seems like the issue happens also in different scenarios with freeze (e.g. #2971). The bug is present only for RN 0.78+, it might be related to React 19.

Fixes #2971.

Details

setImmediate creates a microtask. Microtasks run after a macrotask, before another interation of the event loop. setTimeout is a regular macrotask. Somehow, when using setImmediate in DelayedFreeze, after pushing 4 modal screens with a header, when the first modal gets freezed, an infinite loop of commits is created (you can see the CPU usage go to max, allocated memory increases). Even though each commit has multiple affectedLayoutableNodes, they don't result in any mutations when diffing the trees. These updates probably result in starvation of the function passed to setInterval.

This bug happens with presentation: 'formSheet' as well but not for other types of modals including presentation: 'pageSheet'! This suggests that there is a problem with UIModalPresentationFormSheet (to which UIModalPresentationAutomatic usually resolves to since iOS 18) but not UIModalPresentationPageSheet (which was the default before iOS 18).

Bug mechanism described step-by-step

Please note that react-navigation freezes currently focused screen and one below (this is necessary for animations).

Step 1
Screen Presentation Freeze Details
Home push false We enforce no freeze with freezeOnBlur: false
Step 2
Screen Presentation Freeze Details
Home push false We enforce no freeze with freezeOnBlur: false
Second modal false The modal isFocused so we don't freeze it
Step 3
Screen Presentation Freeze Details
Home push false We enforce no freeze with freezeOnBlur: false
Modal1 modal false The modal isBelowFocused so we don't freeze it
Modal2 modal false The modal isFocused so we don't freeze it
Step 4
Screen Presentation Freeze Details
Home push false We enforce no freeze with freezeOnBlur: false
Modal1 modal false The modal will be freezed after next render (because of DelayedFreeze)
Modal2 modal false The modal isBelowFocused so we don't freeze it
Modal3 modal false The modal isFocused so we don't freeze it
Step 5
Screen Presentation Freeze Details
Home push false We enforce no freeze with freezeOnBlur: false
Modal1 modal true
Modal2 modal false The modal will be freezed after next render (because of DelayedFreeze)
Modal3 modal false The modal isBelowFocused so we don't freeze it
Modal4 modal false The modal isFocused so we don't freeze it

This is the moment when infinite loop happens.

Changes

  • change setImmediate to setTimeout(fn, 0) in DelayedFreeze
  • add Test2963 test screen

Screenshots / GIFs

Before

before_2963_fhd.mov

After

after_2963_fhd.mov

Test code and steps to reproduce

Open Test2963 in example app.

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

@kligarski kligarski marked this pull request as ready for review June 5, 2025 10:12
@kligarski kligarski requested a review from kkafar June 5, 2025 10:12
@kligarski kligarski changed the title fix(iOS): bug with freezing modals fix(iOS): bug with freezing Jun 9, 2025
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, did some sanity checks & it seems we're ok.

I do not have much confidence though, since we do not understand why the infinite loop in react is being triggered by calling setImmediate there.

I'm for making an exception here & proceeding with this change. Since it seems that it fixes serious issues.

@kligarski kligarski changed the title fix(iOS): bug with freezing fix: schedule delayed freeze with setTimeout to prevent React infinite loop Jun 9, 2025
@kligarski kligarski merged commit 45ca50b into main Jun 9, 2025
7 of 10 checks passed
@kligarski kligarski deleted the @kligarski/fix-modal-freeze-ios branch June 9, 2025 10:22
@G2Jose
Copy link

G2Jose commented Jun 10, 2025

Do you know when this will get published on npm @kligarski @kkafar? I'm running into this issue which might be related #2971 (comment)

@kligarski
Copy link
Contributor Author

Hi, @G2Jose. Replied to both of your comments related to the issue here: #2971 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory Leak and Performance Issues with freezeOnBlur in Bottom Tab Navigation
3 participants