Closed Bug 586522 Opened 12 years ago Closed 12 years ago
Leaks in Tab Candy
So, I have determined the cause of the leaks. We basically do not free any of the things that we allocate when we're done! STR: make -C objdir mochitest-browser-chrome TEST_PATH=browser/base/content/tests/tabview It leaks 1,137.611 bytes on my system, consistently. Unfortunately, the fix is not as easy as I would have expected. We basically need to catch the unload event, tell to UI object to release all of its references, which in turn should tell its underlying objects to do the same, and so on. This kind of requires one to be very familiar with the internals of Tab Candy, but until this is done, we won't be abelt o land the code on mozilla-central.
This patch is untested, but it might be useful.
Any reason why cycle collector isn't catching these circular references?
Seems to have gotten rid of the leaks according to tinderbox logs. Yay!
Assignee: nobody → raymond
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to comment #2) > Any reason why cycle collector isn't catching these circular references? Can we get a followup for this?
(In reply to comment #5) > (In reply to comment #2) > > Any reason why cycle collector isn't catching these circular references? > > Can we get a followup for this? Possibly contrary to popular belief -- certainly contrary to the implication of the initial question here -- the XPCOM cycle collector does not magically find all garbage cycles and free them. Please make sure to mention this any time someone asks "why a cycle isn't being freed". We do not have a 100% reliable cycle-finding general garbage collector here. That's a larger task than what we added. The XPCOM cycle collector can find some cycles, sometimes, if all the classes involved support the cycle collection interface and all those classes implement it completely and correctly. If you have any incorrect or partial implementations, or missing implementations, you may wind up with a node in a garbage cycle that you can't account for one of the reference counts for. Then it will not be classified as garbage -- it is still plausibly live -- and will therefore not be collected. The cycle collector is conservative in cases like this; speculatively freeing memory it can't prove to be garbage Would Be Very Bad. Off the top of my head, I don't know which classes involved weren't participating properly, if any. It's also possible that there's a bug in the collector itself. But the first place to check -- if you want a complete explanation -- would be the set of classes making up the cycle(s) you found leaking. I also don't know what the state of the art is in terms of our diagnostics for enumerating candidate cycles that can't be conclusively proven to be garbage. It's been a few years since I've looked at any of the code involved.
I've only interacted with the cycle collector from the XPCOM side of things (making sure that XPCOM classes participate in cycle collection correctly), but what happens when JS objects enter the picture? Do JS objects also take part in cycle collection? Also, is there an easy way to detect which objects failed to participate in the cycle detection correctly by backing out these patches locally and trying to debug things (CCing roc since he's been working on a cycle collector graph which might be useful here).
In bug 497808 there is a patch which will enable dumping of the cycle collector's graph in debug builds. In fact I'm right now extending it so we can ship it in release builds as well.
(In reply to comment #7) > Do JS objects also take part in cycle collection? Yes, they do. The implementation uses JS GC code to navigate the JS part of the object graph and find the reference counts for them. The implementation could be improved as in many cases the reference count could be avoided but this is just an optimization detail.
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy. Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
You need to log in before you can comment on or make changes to this bug.