412527 - bm-xserve11 memory leak unexpectedly dropped?

Status: RESOLVED WORKSFORME

(Toolkit :: Add-ons Manager, defect)

Description

[Justin Dolske [:Dolske]]

The test bustage fix to nsExtensionManager.js.in for bug 311008 seems to have had the unanticipated effect of dropping Lk (trace-malloc leaks) on OS X by about 65K.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsExtensionManager.js.in&branch=&root=/cvsroot&subdir=/mozilla/toolkit/mozapps/extensions/src&command=DIFF_FRAMESET&rev1=1.268&rev2=1.269

Apparently |getDirNoCreate("XREUSysExt", [gApp.ID]);| was throwing or returning null, due to the earlier checkin of the main part of bug 311008. The bustage fix wrapped a bit of code in try/catch/if, such that it's not being run now. That implies either the |new DirectoryInstallLocation()| or |InstallLocations.put()| was the cause of the leak.

So, this leak presumably exists on other platforms, and perhaps in other similar call sites.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

> Apparently |getDirNoCreate("XREUSysExt", [gApp.ID]);| was throwing or returning
> null, due to the earlier checkin of the main part of bug 311008. The bustage
> fix wrapped a bit of code in try/catch/if, such that it's not being run now.
> That implies either the |new DirectoryInstallLocation()| or
> |InstallLocations.put()| was the cause of the leak.

Except those were all brand new calls, and couldn't have been leaking before I added them.  The subsequent try/catch was only necessary when running the test suite inside xpcshell.  Inside the browser, the code does not throw at all.

Comment 2

[Dave Townsend [:mossop]]

Yeah the bugfix only affects code that was checked in a short time before and didn't touch Lk at all. I'd be more inclined to believe that this was the checkin for bug 411327, though why it took a cycle to take effect I'm not sure.

Comment 3

[Justin Dolske [:Dolske]]

Hmm, yeah, I don't really understand what happened here.

Test data:

Run   Time                      Lk      CVS Pull Time
----- -------------------       ------- -------------------------------
20854 2008:01 15:07:35:38	1004623	MOZ_CO_DATE=2008:01:15 07:27:00
20855 2008:01 15:07:45:23	1004954	MOZ_CO_DATE=2008:01:15 07:37:00
20856 2008:01 15:07:56:15	1003343	MOZ_CO_DATE=2008:01:15 07:47:00
20857 2008:01 15:08:19:38	1004941	MOZ_CO_DATE=2008:01:15 08:07:00
20858 2008:01 15:08:36:52	933674	MOZ_CO_DATE=2008:01:15 08:29:00

Run #20857 (the last with the higher leak, which has been at that level for a long time) claims to have a cvs pull date of 8:07am, and #20858 (first with the lower leak) claims to have a pull date of 8:29am. The only change in this range was the bustage fix. 

Checkins around this time:

2008-01-15 07:06 -- bug 404726
2008-01-15 07:14 -- bug 311008
2008-01-15 07:50 -- bug 411327 (checkin start)
2008-01-15 07:57 -- bug 411327 (checkin finish)
2008-01-15 08:28 -- bug 311008 (bustage fix to nsExtensionManager.js.in)
2008-01-15 08:46 -- bug 410927 reverted
2008-01-15 09:38 -- (next checkin, unrelated in Rhino)

So, I'd guess that something in the tests is confused, and that whatever dropped the leak count was either the main checkin for 311008 or 411327. I didn't look at those in detail, but I'm less surprised that a leak would have been fixed there than by the bustage fix.

Given that we're actually leaking *less* now, I don't feel too inclined to look into this deeper. So, closing as WFM.