Technically these fields should have been cycle collected even before the conversion to WebIDL, but I suspect the wrapper caching of File in bug 1047483 made this more prone to leaking. This results in shutdown leaks of windows and documents from content/base/test/test_bug782342.html when running Mochitest-1 in e10s mode. I did a brief audit of fields with strong references to File and these looked like the only two that need to be added. I should also go over XHR to see if anything else is missing.

I've locally confirmed that bug 1047483 is causing the M1 leak I'm seeing with E10s.

Unfortunately, this does not actually fix the leak. This is the rooting path for one of the leaks: 0x7f33948587a0 [File] --[mParent]--> 0x7f33944c2000 [nsGlobalWindow #470 inner http://mochi.test:8888/tests/content/base/test/345339_iframe.html] --[mPerformance]--> 0x7f339681d700 [DOMEventTargetHelper http://mochi.test:8888/tests/content/base/test/345339_iframe.html] --[mParentPerformance]--> 0x7f33947bb040 [DOMEventTargetHelper http://mochi.test:8888/tests/content/base/test/test_bug345339.html] --[mWindow]--> 0x7f33944be400 [nsGlobalWindow #468 inner http://mochi.test:8888/tests/content/base/test/test_bug345339.html] Root 0x7f33948587a0 is a ref counted object with 1 unknown edge(s). known edges: 0x7f3395459c80 [FragmentOrElement (xhtml) input id='file' http://mochi.test:8888/tests/content/base/test/345339_iframe.html] --[mFiles[i]]--> 0x7f33948587a0 0x7f339545a5e0 [FragmentOrElement (xhtml) input id='file' http://mochi.test:8888/tests/content/base/test/345339_iframe.html] --[mFiles[i]]--> 0x7f33948587a0 Maybe this gives somebody some ideas? The presence of performance stuff in this and other paths suggests it is related to the underlying problem that resulted in bug 1064706 getting backed out.

Based on the test case I think the problem is that nsDirectoryService needs to be cycle collected. Fun, fun! Hopefully it doesn't actually need to use threadsafe refcounting. I'll file a new bug for that and just leave this one as is.

Cycling collecting a service doesn't do any good, it lives until shutdown anyways. Things with process lifetime should not be allowed to hold strong references to things with more transitory lifetimes.

I filed bug 1081453 for doing something about nsDirectoryService.

Comment on attachment 8503523 [details] [diff] [review] traverse and unlink Review of attachment 8503523 [details] [diff] [review]: ----------------------------------------------------------------- I need to do a try push with this, but it should be okay. M1 at least seems okay with it, when I run it locally. This does not help the test_bug345339.html leaks (bug 1081453) but seems to fix 1.6MB or so of leaks on e10s M1.

[Tracking Requested - why for this release]: possible bad leaks

We should traverse/unlink mBlobSet too, because that can contain CCed FileImpl objects. Can you file a follow up for this? What I think we should do is some kind of travese()/unlink() methods in BlobSet where check if the FileImpl objects have to be CCed.

(In reply to Andrea Marchesini (:baku) from comment #8) > We should traverse/unlink mBlobSet too, because that can contain CCed > FileImpl objects. > Can you file a follow up for this? I noticed that, but I wasn't sure how the CC set up was supposed to work. But I'll ask in the follow up bug when I file it. try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1d69baf3ddd3

> I noticed that, but I wasn't sure how the CC set up was supposed to work. > But I'll ask in the follow up bug when I file it. Basically any FileImpl has a IsCCed() method. If it returns true, we should call Unlink() and Traverse().

Filed bug 1083349 for blob set.

Comment on attachment 8503523 [details] [diff] [review] traverse and unlink Approval Request Comment [Feature/regressing bug #]: bug 1047483 [User impact if declined]: possible bad memory leaks [Describe test coverage new/current, TBPL]: TBPL [Risks and why]: low, pretty standard fix [String/UUID change made/needed]: none