Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only reset abortcontroller ref if captured instance is same as ref #393

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EtherZa
Copy link

@EtherZa EtherZa commented Feb 7, 2024

#302 describes a race condition where an aborted request resets the abort controller ref even though a subsequent request has been issued and already replaced the ref.

Code sandbox: https://codesandbox.io/p/sandbox/react-hooks-usefetch-forked-xsdrhl?file=%2Fsrc%2Findex.js

Steps to reproduce:

  1. Open sand box
  2. View browser network traffic (ctrl + i / Option + ⌘ + I / select network)
  3. Repeatedly click "Click Me" before previous request has completed
  4. Note that approximately every second request is aborted while the others complete

Cause:
On completing a request (normal or aborted), the captured AbortController ref (controller.current) is set to undefined in the finally block. If a request is aborted and then immediately resubmitted, this results in a race condition where:

  1. Request A is started
  2. AbortController for request A is captured
  3. Request A abort is initiated via abort()
  4. Abort signal is sent
  5. Request B is started
  6. AbortController for request B is captured
  7. Request A completes and sets captured controller ref to undefined
  8. Request B abort is initiated via abort()
  9. Abort signal has been lost by 8 and cannot be issued
  10. Request B completes

Resolution:
Only set the controller ref to undefined if the ref is still set to the current controller instance

Note:
This PR makes no attempt to resolve the underlying race condition. It proposes a solution to allow for the last issued request to be aborted while potentially managing the race condition through other means.

@freshollie
Copy link

This solves an issue we were experiencing with this library. @alex-cory I'm sure others would benefit from merging

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

Successfully merging this pull request may close these issues.

2 participants