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

Bugfix: "Captured" updates on legacy queue #18265

Merged
merged 3 commits into from Mar 11, 2020

Commits on Mar 10, 2020

  1. Bugfix: "Captured" updates on legacy queue

    This fixes a bug with error boundaries. Error boundaries have a notion
    of "captured" updates that represent errors that are thrown in its
    subtree during the render phase. These updates are meant to be dropped
    if the render is aborted.
    
    The bug happens when there's a concurrent update (an update from an
    interleaved event) in between when the error is thrown and when the
    error boundary does its second pass. The concurrent update is
    transferred from the pending queue onto the base queue. Usually, at this
    point the base queue is the same as the current queue. So when we
    append the pending updates to the work-in-progress queue, it also
    appends to the current queue.
    
    However, in the case of an error boundary's second pass, the base queue
    has already forked from the current queue; it includes both the
    "captured" updates and any concurrent updates. In that case, what we
    need to do is append separately to both queues. Which we weren't doing.
    
    That isn't the full story, though. You would expect that this mistake
    would manifest as dropping the interleaved updates. But instead what
    was happening is that the "captured" updates, the ones that are meant
    to be dropped if the render is aborted, were being added to the
    current queue.
    
    The reason is that the `baseQueue` structure is a circular linked list.
    The motivation for this was to save memory; instead of separate `first`
    and `last` pointers, you only need to point to `last`.
    
    But this approach does not work with structural sharing. So what was
    happening is that the captured updates were accidentally being added
    to the current queue because of the circular link.
    
    To fix this, I changed the `baseQueue` from a circular linked list to a
    singly-linked list so that we can take advantage of structural sharing.
    
    The "pending" queue, however, remains a circular list because it doesn't
    need to be persistent.
    
    This bug also affects the root fiber, which uses the same update queue
    implementation and also acts like an error boundary.
    
    It does not affect the hook update queue because they do not have any
    notion of "captured" updates. So I've left it alone for now. However,
    when we implement resuming, we will have to account for the same issue.
    acdlite committed Mar 10, 2020
    Copy the full SHA
    12b3075 View commit details
    Browse the repository at this point in the history

Commits on Mar 11, 2020

  1. Ensure base queue is a clone

    When an error boundary captures an error, we append the error update
    to the work-in-progress queue only so that if the render is aborted,
    the error update is dropped.
    
    Before appending to the queue, we need to make sure the queue is a
    work-in-progress copy. Usually we clone the queue during
    `processUpdateQueue`; however, if the base queue has lower priority
    than the current render, we may have bailed out on the boundary fiber
    without ever entering `processUpdateQueue`. So we need to lazily clone
    the queue.
    acdlite committed Mar 11, 2020
    Copy the full SHA
    1a55692 View commit details
    Browse the repository at this point in the history
  2. Add warning to protect against refactor hazard

    The hook queue does not have resuming or "captured" updates, but if
    we ever add them in the future, we'll need to make sure we check if the
    queue is forked before transfering the pending updates to them.
    acdlite committed Mar 11, 2020
    Copy the full SHA
    62de1b3 View commit details
    Browse the repository at this point in the history