1359201 - Add a nukeable proxy for HttpBaseChannel::mCallbacks

Status: RESOLVED WONTFIX

(Core :: Networking: Proxy, enhancement, P3)

Description

[Andrew McCreight [:mccr8]]

Billm suggested that we could have a nukeable proxy for C++ references "outside" the DOM to references in the DOM. A proxy would be associated with a particular window, and would act like a strong reference until the window got nuked, on page close, when it would get cleared out. This is like a weak reference, except with a specified lifetime. This could prevent many kinds of ghost windows.

While this would be a big project to convert everything, I think implementing this just for HttpBaseChannel::mCallbacks could be a big help. Channels aren't cycle collected, and these callbacks can be implemented in JS, so it is very easy to create a cycle through content, leaking the page forever. We also don't seem to test the case of JS implemented mCallbacks very well, but they are heavily used in addons like AdblockPlus, so any bugs there can cause really bad leaks for many users.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This doesn't seem completely insane.

That said, the channel contract is supposed to be that once you call AsyncOpen the channel will drop the mCallbacks pointer at some point; either before returning from AsyncOpen (on failure return) or after calling OnStopRequest (on successful return from AsyncOpen).  Now channel's haven't always _fulfilled_ this contract...

Comment 2

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 4

[Andrew McCreight [:mccr8]]

Bug 1658571 is an example of bad leaks due to callbacks. I think that's nsInputStreamChannel, which is not a subclass of HttpBaseChannel.

Comment 5

[Andrew McCreight [:mccr8]]

I implemented something similar to this for service worker references to promises in bug 1409115, though it looks like the uses of it got removed at some point.

Comment 6

[BMO Automation]

Moving bug to Core/Networking: Proxy.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Do you still think this would be useful?

Comment 8

[Andrew McCreight [:mccr8]]

I think the same basic problem is present, but I personally don't remember seeing any of these issues for a long time, so I think it is fine to close this for now.