393993 - Run mochitests with leak-gauge.pl logging, and make the tests fail when windows/documents/docshells leak

Status: RESOLVED WONTFIX

(Testing :: Mochitest, defect)

Description

[Jeff Walden [:Waldo]]

If we know we don't leak on something, we should do everything possible to ensure we don't ever leak on it again.  Also, we need a way to write tests which pass when no leak happens and fail when a leak does happen, when those leaks are easily detectable, as is true of docshell/window/document leaks.

The biggest obstacle to this may be fixing the leaks in our current tests.  In writing a leak test for bug 393974 I found that without that patch or its test we leak about:blank just running the tests in dom/tests/mochitest/bugs; if we leak so easily on running just one small directory of tests, I shudder to think how bad it must be over all the mochitests we run.  I'll post some stats on that later today, after I've had a chance to use a hacked runtests.pl to see just how bad the story is.

Comment 1

[Rob Campbell [:rc] (:robcee)]

Will this have any impact on the running time of the mochitests themselves? I'm reluctant to slow down the unittests, but if it catches memory leaks, it might be worth a hit.

Comment 2

[Jeff Walden [:Waldo]]

There's a little impact, but not much; basically, it just causes a few PR_LOG() macro instances to actually do something, namely print logging like "new document at <address>", "document destroyed at <address>", etc.  It shouldn't have significant effect on test performance.  Running all the mochitests, this generates a file of only 34337 lines, and since presumably the logging doesn't disable buffering, it should do its logging reasonably efficiently.

Also, the news on the leak front, at least on Mac, isn't as bad as I'd expected.  It looks like we only leak the one inner/outer window pair I happened to find, five documents (all the same one, which should make finding the leak easier), and no docshells.  This is the output for a log generated from running Mochitests with all the logging enabled:


207:~/moz/builds/trunk/_tests/testing/mochitest jwalden$ perl \
>   ~/moz/inflight/leak-gauge.pl nspr.log
Leaked outer window 326bbba0 at address 326bbba0.
Leaked inner window 39912220 (outer 326bbba0) at address 39912220.
 ... with URI "about:blank".
Leaked document at address 4000ede0.
 ... with URI "http://localhost:8888/tests/layout/style/test/foo.xml".
Leaked document at address 400ac1d0.
 ... with URI "http://localhost:8888/tests/layout/style/test/foo.xml".
Leaked document at address 40243790.
 ... with URI "http://localhost:8888/tests/layout/style/test/foo.xml".
Leaked document at address 365104c0.
 ... with URI "http://localhost:8888/tests/layout/style/test/foo.xml".
Leaked document at address 375d2460.
 ... with URI "http://localhost:8888/tests/layout/style/test/foo.xml".
Summary:
Leaked 2 out of 3865 DOM Windows
Leaked 5 out of 3193 documents
Leaked 0 out of 1291 docshells

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Somewhat-tested patch (buildbot changes untested)

This works enough that running Mochitests will print out success or failure messages.  The buildbot changes are untested.

Eli has the XBL leaks under control, and it sounds like his patch in bug 394052 fixes all of them.  I'll start looking into the window leak; given that I seem to see it on any test of tests I run (but just opening/closing without running tests doesn't leak), I have a suspicion it's a leak caused by Mochitest itself, but we'll see after I pull out some other leak tools and figure out what's wrong.

I haven't looked to see how browser or chrome tests leak, if they do; they'll need similar fixup in order to land this patch or something like it.

Comment 4

[Rob Campbell [:rc] (:robcee)]

so, if I'm reading this correctly, it would require us to have leak-gauge.pl installed somewhere in the system's path for it to work. While not a huge problem, it would make setup easier if we could just bundle it into the mochitest directory (or somewhere else like or testing/tests).

Do we want to leave the leak file in the running profile? In the case of mochitest it gets wiped out on next run. This is probably ok, but I'm just wondering if there's any reason to want to keep these logs for analysis?

The buildbot changes look good (and slightly optimized from where they are now, to boot). :)

Comment 5

[Jeff Walden [:Waldo]]

(In reply to comment #4)
> so, if I'm reading this correctly, it would require us to have leak-gauge.pl
> installed somewhere in the system's path for it to work.

No; see the Makefile.in change.  That copies it to the same location as httpd.js and friends, and once it's there we can use it just as easily as any of the other files copied there.

Comment 6

[Jeff Walden [:Waldo]]

That said, I think it would be awesome if we just preprocessed an absolute-path $topsrcdir into the script so we wouldn't even need to copy some things, but I can only stomach Perl every so often.  I really should have just rewritten the script in Python when I had the chance.  ;-)  :-\

Comment 7

[Jeff Walden [:Waldo]]

Attachment: Now with infinitely more Windows support

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Clearing the nom -- testing stuff isn't going to block/not-block the release, though I'd encourage you to use the priority field to indicate important bugs.

Comment 9

[Jeff Walden [:Waldo]]

This task is essentially superseded by the leak logging in runtests.py, so I'm marking this WONTFIX.  That code catches leaks of any refcounted objects, not just documents/windows/docshells in trace-refcnt builds (which constitute everything except release builds), so we just need runtests.py on tinderboxen to address the problem this bug addressed...

Comment 10

[(not currently active) Ted Mielczarek]

Mass move of the rest of the Mochitest bugs from Core:Testing to Testing:Mochitest. Filter on MochitestMassMove to ignore.