Lately we've taken more notice of "leaks", where we hold on to an object longer than we should (but not past shutdown). I was thinking about ways to write tests for these kinds of bugs since our traditional tests won't catch these and came up with the following. Provide two new global functions (on SpecialPowers? as an xpcom object?): registerObject: takes an object that supports weak references (which includes the big ones, documents and windows), stores a weak ref, and hands back some sort of identifier. isObjectDead: takes the identifier mentioned above, looks up the weak reference, forces a GC/CC, and then sees if the object is still alive via the weakref. This won't let us test everything, but it would get the big objects. Jesse, would this help you fuzz for this kind of stuff? Thoughts, comments?

For regression tests, this sounds excellent. It could be used to construct an automated test for bug 610956, for example. I'd keep the GC/CC calls separate from isObjectDead. It can be too much, if you're hoping to check isObjectDead for a bunch of objects at once, and want it to be fast. It can be too little, if collecting an object requires multiple CCs or a return to the event loop between CCs (e.g. bug 610166). You might want to provide an async GC/CC API to guarantee that conservative stack scanning won't incorrectly keep things alive. For fuzzing, I'm not sure this would be useful. With all the crazy stuff going on, there's no obvious point at which a given {frame, tab, browser window, page in session history} "should be dead". I can close all browser windows (or navigate away from the main fuzz page with bfcache disabled? - see bug 630072 comment 53), but then I'm happier counting the remaining windows and documents than asking whether specific windows and documents are alive.

This sounds very similar in philosophy to David Baron's (I think) ExplainLiveExpectedGarbage for the CC. I'm not sure exactly how that works, or how effective he found it to be.

> You might want to provide an async GC/CC API to guarantee that conservative > stack scanning won't incorrectly keep things alive. jdm added such an API in bug 661927 :)

What's the status of this?

I have a patch in my queue but ran into a bug that was hard to track down and then got whisked away to other things. I'll see if I can figure it out soon.

Heh, and with a few weeks distance it was pretty easy to find the bug.

Comment on attachment 556571 [details] [diff] [review] Patch >+already_AddRefed<mozILifetimeTester> >+GetLifetimeTester() >+{ >+ if (!sLifetimeTester) { >+ // The service manager will hold us alive until shutdown. >+ sLifetimeTester = new mozLifetimeTester(); >+ bool inited = sLifetimeTester->Init(); >+ NS_ENSURE_TRUE(inited, nsnull); >+ } I'm not a fan of this function, for a few reasons: - it holds the object without adding a reference until near the end - it has an unnecessary QI - if Init() ever does fail, this'll return null the first time and then non-null the rest (since you'll never actually call the destructor, because of the first problem) Do you really need a moz prefix on a class inside mozilla::testing::? That's not the convention we've been using. (And I'm not crazy about moz prefixes in general; I'd rather just stick with ns.) Also, could you make ~mozLifetimeTester() explicitly private rather than relying on the default? In RegisterObject, why is it an error to register the same object twice. That seems like unnecessary pain -- why not just return the ID? Using the pointer as the ID doesn't seem valid -- it's subject to pointer replay (though if you hit it you'll hit the NS_ERROR_UNEXPECTED you currently have, but I think that's a bad idea -- could still lead to random test failures). I think you need to generate and track your own IDs. (In that case, RegisterObject could just always give out a new ID even if the object is already registered.) If you do that you could even use an array rather than a hashtable. In the IDL, don't have an empty comment. Either have none or say something. Preferably say something useful. You should ask bsmedberg, as xpcom owner, if he's ok with the file location and the mozilla::testing:: namespace, at the very least. review- because of the pointer replay issues