Closed Bug 470710 Opened 12 years ago Closed 12 years ago

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

Categories

(SeaMonkey :: General, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
seamonkey2.0b1

People

(Reporter: sgautherie, Assigned: stefanh)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak)

Attachments

(1 file)

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.
Flags: wanted-seamonkey2?
See also my comments in http://home.kairo.at/blog/2008-12/zarro_leaks_found - could be connected with non-browser windows...
(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...
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!
Keywords: helpwanted
Summary: Investigate the MacOSX-specific "mochitests" leaks → Narrow down the MacOSX-specific "reftest"s and "mochitest"s leak
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.
Blocks: 469518
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.
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).
Depends on: 470709
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.
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?
Summary: Narrow down the MacOSX-specific "reftest"s and "mochitest"s leak → [MacOSX] Fix "reftest"s and "mochitest"s 1,112 bytes leak caused by hiddenWindow.xul
Whiteboard: [See comment 7]
(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.
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?
Depends on: 489985
(In reply to comment #10)
> Ben, any idea?

I filed bug 489985, as this issue affects Windows too.
Attached patch Fix leaksSplinter Review
Calling Shutdown() on unload in the hiddenwindow seems to fix the leaks for me.
Assignee: nobody → stefanh
Status: NEW → ASSIGNED
Attachment #374477 - Flags: review?(neil)
Attachment #374477 - Flags: review?(neil) → review+
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.
(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.
(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.
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 :)
--> Fixed :)
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: helpwanted
Resolution: --- → FIXED
Whiteboard: [See comment 7]
Target Milestone: --- → seamonkey2.0b1
V.Fixed, per tinderbox

KaiRo, can you reduce the MacOSX thresholds to 0? :-)
Status: RESOLVED → VERIFIED
Flags: wanted-seamonkey2? → in-testsuite-
Hardware: x86 → All
No longer blocks: 469518
Depends on: 469518
(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.
(In reply to comment #18)
> KaiRo, can you reduce the MacOSX thresholds to 0? :-)

http://hg.mozilla.org/build/buildbot-configs/rev/91521f3b3bf1
You need to log in before you can comment on or make changes to this bug.