Closed Bug 1440941 Opened 6 years ago Closed 6 years ago

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

Categories

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

58 Branch
defect
Not set
normal

Tracking

()

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

People

(Reporter: soeren.hentzschel, Assigned: baku)

References

Details

(Keywords: crash)

Attachments

(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:
https://crash-stats.mozilla.com/report/index/d9227073-3d78-4205-85f3-2b6570180224

(@ 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 amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bfc4dd20e01
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?
https://hg.mozilla.org/mozilla-central/rev/1bfc4dd20e01
Status: NEW → RESOLVED
Closed: 6 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]
fetch_crash.patch

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.
Status: RESOLVED → VERIFIED
Comment on attachment 8954123 [details] [diff] [review]
fetch_crash.patch

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: https://archive.mozilla.org/pub/firefox/candidates/59.0b14-candidates/build1/.
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.