If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsUserFontSet/nsFontFaceLoader do not check result of nsIChannel::AsyncOpen, resulting in leaks

RESOLVED FIXED in Firefox 19

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nmaier, Assigned: nmaier)

Tracking

unspecified
mozilla20
Points:
---

Firefox Tracking Flags

(firefox19 fixed, firefox20 fixed, b2g18 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
nsUserFontSet will set up a channel and CORS proxy, but fail to tear down the references should nsIChannel::AsyncOpen fail, thus leaking the instance itself and referenced objects like said channel.

Code in question:
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsFontFaceLoader.cpp#383
Note how the code will call fontLoader->DropChannel should listener->Init() fail, but not when channel->AsyncOpen fails a couple of lines down the block.

This blocks (mochikit-o orange due to leaks) my fix for bug 719180, which, as a side effect, will make the channel fail early on AsyncOpen in some cases where before it would only fail later via OnStopRequest.
However, even without the fix from bug 719180 there are cases where AsyncOpen might fail early and hence has nsUserStyleSet leak.
(Assignee)

Comment 1

5 years ago
Created attachment 682893 [details] [diff] [review]
Fix nsUserFontSet not dropping the channel when nsIChannel::AsyncOpen fails

This will correctly call DropChannel() not only if listener->Init() fails, but also when channel->AsyncOpen() does fail.

Green try with that patch (same code, different name):
https://tbpl.mozilla.org/?tree=Try&rev=aa22b56b4cfb
The other changesets can be considered tests; would be leak-orange on mochikit-o without this patch.
Attachment #682893 - Flags: review?(bzbarsky)
Comment on attachment 682893 [details] [diff] [review]
Fix nsUserFontSet not dropping the channel when nsIChannel::AsyncOpen fails

r=me
Attachment #682893 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 3

5 years ago
Checkin-needed. Green try: https://tbpl.mozilla.org/?tree=Try&rev=aa22b56b4cfb
Assignee: nobody → MaierMan
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b43d9de3c05
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5b43d9de3c05
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment on attachment 682893 [details] [diff] [review]
Fix nsUserFontSet not dropping the channel when nsIChannel::AsyncOpen fails

See bug 719180 comment 43
Attachment #682893 - Flags: approval-mozilla-b2g18?
Attachment #682893 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Attachment #682893 - Flags: approval-mozilla-b2g18?
Attachment #682893 - Flags: approval-mozilla-b2g18+
Attachment #682893 - Flags: approval-mozilla-aurora?
Attachment #682893 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/db9a8576a8fd
https://hg.mozilla.org/releases/mozilla-b2g18/rev/f1c819ce835c
status-b2g18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
You need to log in before you can comment on or make changes to this bug.