Closed Bug 1440941 Opened 5 years ago Closed 5 years ago

Aborting fetch with a lot of requests causes browser crash (@mozilla::dom::FetchDriver::OnStartRequest)


(Core :: DOM: Core & HTML, defect)

58 Branch
Not set



Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 --- fixed
firefox60 --- verified


(Reporter: soeren.hentzschel, Assigned: baku)



(Keywords: crash)


(2 files)

I am working on a new version of my WebExtension Bookmarks Organizer. One change is that I want to abort the request after x seconds.

There are no problems with the old code which doesn't abort:

const response = await fetch(bookmark.url, {
  cache : 'no-store',
  credentials : 'include',
  method : method

But once I change the code to the following:

const controller = new AbortController();
const signal = controller.signal;

setTimeout(() => controller.abort(), 3000);

const response = await fetch(bookmark.url, {
  cache : 'no-store',
  credentials : 'include',
  method : method,
  signal : signal

… Firefox crashes on my profile with more than 5,000 bookmarks after a few seconds. On a new profile with only 9 bookmarks there is no crash. It happens in every version from Firefox Stable 58 (tested via about:debugging) to Firefox 60 Nightly.

Here is one of my crash reports:

(@ mozilla::dom::FetchDriver::OnStartRequest)

ni? :baku because you implemented aborting fetch requests.

Do you have any idea what's going on there? Is there anything what I can do to help finding the cause? Do you need a backup of my bookmarks? (the crash is reproducible on my system when exporting the bookmarks and importing in a new profile).

I attached a preview version of the new add-on version. You can open the add-on interface via the toolbar button. Pressing the check bookmarks button on the bottom with the default option in the dropdown on the bottom left causes the crash after a few seconds for me.
Flags: needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8954123 - Flags: review?(bkelly)
Attachment #8954123 - Flags: review?(bkelly) → review+
Pushed by
FetchDriver should check if the operation has been already aborted when OnStartRequest is called, r=bkelly
Thank you for the very quick patch! I will test once it's in Firefox Nightly. If it works and won't be backed out for whatever reason, is there a chance that this bugfix will be uplifted to Firefox 59 Beta or is it too risky because it's already so late in the beta cycle?
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Andrea, can you uplift to beta since its a pretty safe fix?
Flags: needinfo?(amarchesini)
Comment on attachment 8954123 [details] [diff] [review]

Approval Request Comment
[Feature/Bug causing the regression]: Abort API
[User impact if declined]: a crash
[Is this code covered by automated tests?]: race condition
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: There is an STR.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low
[Why is the change risky/not risky?]: When OnStartRequest is called, this patch introduces a new check about mChannel being already nullified.
[String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8954123 - Flags: approval-mozilla-beta?
I can verify the fix works for the STR in comment #0.
Comment on attachment 8954123 [details] [diff] [review]

Minor API fix, verified in nightly, looks fine for uplift for beta 14.
Attachment #8954123 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Sören, could you please confirm whether this is fixed on Firefox 59 Beta 14 as well?

You can find the build here:
Flags: needinfo?(cadeyrn)
I can confirm it's fixed in Firefox 59 Beta 14.
Flags: needinfo?(cadeyrn)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.