470710 - [MacOSX] Fix "reftest"s and "mochitest"s 1,112 bytes leak caused by hiddenWindow.xul

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

Bug 450983 comment 39:
{
From  Robert Kaiser   2008-12-20 14:37:24 PST

OS X leaks 1112+8 on mochitest and 1112+0 on mochichrome, and even still leaks
slightly more than the others on browser chrome tests - the 1112 seems to be
some magtc number across the machines ;-)
}

Linux and Windows leak 8 / 0 / 1112. (bug 458781 / - / bug 470709)

MacOSX leaks more: +1112 / +1112 / +120.
We need someone with a MacOSX to track down these additional leaks.

Comment 1

[Robert Kaiser]

See also my comments in http://home.kairo.at/blog/2008-12/zarro_leaks_found - could be connected with non-browser windows...

Comment 2

[Serge Gautherie (:sgautherie)]

(mid-air collision, but still:)

It might be interesting (as a hint) to check whether bug 391318 comment 8 workaround "fixes" these leaks (as it does for bug 470709) or not...

Comment 3

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1238612305.1238618294.27235.gz&fulltext=1
MacOSX 10.4 comm-central dep unit test on 2009/04/01 11:58:25

SeaMonkey is now reporting leaks for "reftest"s too:
the MathML part is Core bug 458844;
the rest looks the "same" as the leak reported by "mochitest"s.

(In reply to comment #0)
> We need someone with a MacOSX to track down these additional leaks.

(see also comment 2)
helpwanted!

Comment 4

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1239883335.1239889982.18475.gz&fulltext=1
OS X 10.4 comm-central unit test on 2009/04/16 05:02:15

KaiRo,
as a follow-up to bug 469518 comment 41,
reftest and crashtest need either be (tracked down and) fixed,
or get a 1112 bytes leak threshold.

Comment 5

[Robert Kaiser]

Neil, can we get bug 470709 fixed in some sane way soon? I think this would fix almost or completely everything seen here as well.

Comment 6

[Robert Kaiser]

OK, so it seems pretty clear that this is happening on Mac because 1) Mac always opens a hidden window, which probably has the View menu with the theme switching stuff on it, and 2) the workaround for the EM leak isn't triggered correctly due to a different failure, as fixed by the latest patch to bug 470709, which just waits for Neil checking it in (let him do it, he should find the time soon).

Comment 7

[Robert Kaiser]

Actually, comment -6 points into the right direction but the reason why the workaround isn't triggered seems to be that while hiddenWindow.xul includes navigatorOverlay.xul and constructs the apply theme submenu, it doesn't have an unload function and never runs the code in navigator.js Shutdown() that works around the leak.

Comment 8

[Serge Gautherie (:sgautherie)]

Since
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1240518372.1240524712.16272.gz
OS X 10.4 comm-central unit test on 2009/04/23 13:26:12
reftest and crashtest have |EXTRA_TEST_ARGS=--leak-threshold=1112|

KaiRo, is it a change you did locally to the box?

Comment 9

[Robert Kaiser]

(In reply to comment #8)
> KaiRo, is it a change you did locally to the box?

Non, I have no idea why it suddenly uses a threshold.

Comment 10

[Serge Gautherie (:sgautherie)]

While I'm happy this turned the box green,
I'm worried this could be a setup/env leak (from mochitest config) caused by the recent code changes?
Ben, any idea?

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to comment #10)
> Ben, any idea?

I filed bug 489985, as this issue affects Windows too.

Comment 12

[Stefan [:stefanh]]

Attachment: Fix leaks

Calling Shutdown() on unload in the hiddenwindow seems to fix the leaks for me.

Comment 13

[Stefan [:stefanh]]

I pushed attachment #374477 [details] [diff] [review] to comm-central (changeset 9ed376b9f6fd). I'll wait a few hours before I close this - so we're sure it's fixed.

Comment 14

[[e-mail] (:bhearsum)]

(In reply to comment #10)
> While I'm happy this turned the box green,
> I'm worried this could be a setup/env leak (from mochitest config) caused by
> the recent code changes?
> Ben, any idea?

I would be very very very surprised those changes somehow caused the application to leak. For posterity, the recent changes are:
http://hg.mozilla.org/build/buildbotcustom/diff/153cc725f37c/unittest/steps.py and http://hg.mozilla.org/build/buildbotcustom/rev/9301fd18edfd. Neither of which have functional changes.

Comment 15

[Robert Kaiser]

(In reply to comment #14)
> I would be very very very surprised those changes somehow caused the
> application to leak.

Ben, I think you misunderstood there, and I agree using "leak" in different ways in the same bug is confusing. ;-)

What Serge meant was that it looks like the mochitest leakThreshold setting, which is using an env var, seems to be "leaking" over to the reftest and crashtest steps that don't have a threshold set in the config but somehow now run with the same one as mochitest-plain.

Comment 16

[Robert Kaiser]

On the actual bug, I can report that all leaks on all Mac tests seem to be fixed with the patch. Stefan, feel free to mark FIXED :)

Comment 17

[Stefan [:stefanh]]

--> Fixed :)

Comment 18

[Serge Gautherie (:sgautherie)]

V.Fixed, per tinderbox

KaiRo, can you reduce the MacOSX thresholds to 0? :-)

Comment 19

[Robert Kaiser]

(In reply to comment #18)
> KaiRo, can you reduce the MacOSX thresholds to 0? :-)

Sure, will do this, but might take a bit to actually get deployed.

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #18)
> KaiRo, can you reduce the MacOSX thresholds to 0? :-)

http://hg.mozilla.org/build/buildbot-configs/rev/91521f3b3bf1