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

Fix: Detect infinite update loops caused by render phase updates #26625

Merged
merged 4 commits into from
Jun 27, 2023

Commits on Jun 27, 2023

  1. Configuration menu
    Copy the full SHA
    d2edee9 View commit details
    Browse the repository at this point in the history
  2. Add an infinite loop guard around sync work loop

    We already have an infinite loop guard that detects unguarded updates,
    like if you repeatedly call `setState` in an effect — the nth+1 update
    will trigger an error. However, a limitation is that it depends on the
    error causing the component to be unmounted. This is usually the case
    but there are ways to break this expectation with undefined behavior,
    like if you call `setState` during the render phase on an different
    component (which is a bad pattern that is always accompanied by a
    warning). Aside from product bugs, if there were a bug in React, it
    might also cause something like this. Whether the bug happens because
    of app code or internal React code, we should do our best to avoid
    infinite loops because they are so hard to debug; even if it's a fatal
    bug and the UI can't recover, we at least need to free up the main
    thread so that it's more likely for the error to be reported.
    
    So this adds a second infinite loop guard around the synchronous
    work loop. This won't cover all possible loops, such as ones triggered
    during a concurrent render, but it should cover the most egregious ones.
    acdlite committed Jun 27, 2023
    Configuration menu
    Copy the full SHA
    067f53b View commit details
    Browse the repository at this point in the history
  3. Disable error recovery mechanism if infinite loop is detected

    If an infinite update loop is caused by a render phase update, the
    mechanism we typically use to break the loop doesn't work. Our mechanism
    assumes that by throwing inside `setState`, the error will caise the
    component to be unmounted, but that only works if the error happens in
    an effect or lifecycle method. During the render phase, what happens is
    that React will try to render the component one more time,
    synchronously, which we do as a way to recover from concurrent data
    races. But during this second attempt, the "Maximum update" error won't
    be thrown, because the counter was already reset.
    
    I considered a few different ways to fix this, like waiting to reset the
    counter until after the error has been surfaced. However, it's not
    obvious where this should happen. So instead the approach I landed on is
    to temporarily disable the error recovery mechanism. This is the same
    trick we use to prevent infinite ping loops when an uncached promise is
    passed to `use` during a sync render.
    
    This category of error is also covered by the more generic loop guard I
    added in the previous commit, but I also confirmed that this change
    alone fixes it.
    acdlite committed Jun 27, 2023
    Configuration menu
    Copy the full SHA
    6adc321 View commit details
    Browse the repository at this point in the history
  4. Detect non-synchronous recursive update loops

    This improves our infinite loop detection by explicitly tracking
    when an update happens during the render or commit phase.
    
    Previously, to detect recursive update, we would check if there was
    synchronous work remaining after the commit phase — because synchronous
    work is the highest priority, the only way there could be even more
    synchronous work is if it was spawned by that render. But this approach
    won't work to detect async update loops.
    acdlite committed Jun 27, 2023
    Configuration menu
    Copy the full SHA
    ae39371 View commit details
    Browse the repository at this point in the history