Closed Bug 795305 Opened 7 years ago Closed 7 years ago

httpchannel::asyncopen failure holds on to callback reference

Categories

(Core :: Networking: HTTP, defect)

16 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: mcmanus, Assigned: mcmanus)

Details

Attachments

(1 file)

a channel that fails AsyncOpen() is done (i.e. it will not call onstartrequest/onstoprequest).. and channel.idl says

"When the channel is done, it must not continue holding references to
this object. [sic the callback]"

AsyncOpen doesn't clear the callback information on failure in the same way we do when the channel completes via OnStopRequest().

I noticed this in the AsyncFetchAndSetIconFromNetwork code when I insturmented asyncopen to return a failure to test a theory on a different bug. That failure resulted in AFASIFN being leaked because of a reference loop - AFASIFN held a ref to the channel and the channel held a ref to AFASIFN (as the callback).. normally the channel released its when it called OSR, but in the asyncopen case the deadlock persisted. 

(you could also argue that AFASIFN should drop the channel when asyncopen() failed, which would be good - but contractually I think the http channel is in the wrong.)

That's the meaningful part of the patch.. the bulk of the patch is just the creation of a helper function that clears all 4 standard referneces (callbacks, listener, etc..) together so we don't miss some in the future.
Attached patch patch 0Splinter Review
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
Attachment #665880 - Flags: review?(jduell.mcbugs)
Attachment #665880 - Attachment is patch: true
Comment on attachment 665880 [details] [diff] [review]
patch 0

Review of attachment 665880 [details] [diff] [review]:
-----------------------------------------------------------------

Good stuff.
Attachment #665880 - Flags: review?(jduell.mcbugs) → review+
https://hg.mozilla.org/mozilla-central/rev/5a159a90e13f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.