Attempts to display blocked resources with no blocking reason leak
Categories
(DevTools :: Netmonitor, defect)
Tracking
(firefox73 fixed)
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Consider this testcase:
<video>
<track src="http://this-is-not-the-site-I-am-hosted-on-I-am-sure" default>
</video>
This testcase leaks when loaded while the network tab in devtools is open. This is easy to see by setting XPCOM_MEM_BLOAT_LOG
to some filename, starting a debug browser, opening the network panel, loading the testcase, then quitting.
This leak is what caused bug 1602090 to get backed out, since all cases that lead to the error message being surfaced there also lead to this leak.
The leak is caused by the fact that in this case the channel is blocked without setting a requestBlockingReason
on the loadinfo. Then we end up in _httpFailedOpening
with a topic of "http-on-failed-opening-request"
. That calls _httpResponseExaminer
, which calls _createNetworkEvent
. The blockedReason
passed in at this point is 0
, which does NOT mean "not blocked"; it just means "whoever blocked this did not provide a reason".
Then createNetworkEvent
does:
if (!event.blockedReason) {
this._setupResponseListener(httpActivity, fromCache);
}
which ends up throwing on this line:
const originalListener = channel.setNewListener(tee);
because at this point the channel is not in a state where you can mess with it's listener: it's already canceled.
Unfortunately, before getting to the exception point _setupResponseListener
creates a cycle that includes the channel. In particular, the NetworkResponseListener is in such a cycle, and the channel has already dropped its notification callbacks and whatnot when it errored out (the OnFailedOpeningRequest
call is the last thing in HttpChannelChild::AsyncOpen
), so nothing is going to break the cycle after this point.
It looks like this code was added in bug 1151368, so needinfoing the people who might know something about it...
Things we could consider doing:
-
HttpChannelChild::AsyncOpen
could re-clear members after dispatching the failure notification, or prevent them from being set after they got cleared earlier, to avoid leaks. -
The content security manager could set a blocking reason when it blocks due to failing SOP checks. That said, there are lots of things that can lead to blocking and I don't know that all the other ones set a reason...
-
The devtools code could avoid losing track of the fact that this is a failed load. In particular, it could replace
!blockedReason
checks withblockedReason != undefined
checks, since0
is a perfectly valid value ofblockedReason
when blocking happened.
Assignee | ||
Comment 2•4 years ago
|
||
Not everything that blocks requests sets a reason other than BLOCKING_REASON_NONE.
Option 3 seems like a fine approach to me.
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bb9911385c92 Network monitor should not lose track of whether the request was blocked. r=ochameau,jryans
Comment 5•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Updated•4 years ago
|
Description
•