Closed
Bug 345530
Opened 18 years ago
Closed 18 years ago
Increased Rlk (leak) on balsa after 2006-07-20
Categories
(Firefox :: General, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 345908
People
(Reporter: ispiked, Unassigned)
Details
(Keywords: memory-leak)
2212 MOZ_CO_DATE=2006:07:20:15:47:00
2328 MOZ_CO_DATE=2006:07:20:21:32:00
Check-ins: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006%3A07%3A20%3A15%3A47%3A00&maxdate=2006%3A07%3A20%3A21%3A32%3A00&cvsroot=%2Fcvsroot
--NEW-LEAKS-----------------------------------leaks------leaks%
nsRunnable 12 -
nsXPCWrappedJS 192 33.33%
XPCWrappedNative 1400 4.17%
I've been told that this sort of leak is due to JS changes. CCing folks who touched JS code during this range.
Comment 1•18 years ago
|
||
The only change I checked in during this time period was a small fix in the microsummary service that declares two variables, assigning the first to the value of a DOM attribute and the second to an array obtained by splitting the first:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/browser/components/microsummaries/src&command=DIFF_FRAMESET&file=nsMicrosummaryService.js.in&rev1=1.28&rev2=1.29&root=/cvsroot
This doesn't seem likely to cause a leak.
Comment 2•18 years ago
|
||
I don't think my checkins in that range would have caused leaks, but it's hard to be sure. I'm somewhat suspicious of Pam's patch, since it adds references from the observer service to the search bar binding, and there have been issues in the past with XBL destructors not firing (though that would likely mean that we're already leaking due to the existing references to the search bar binding). Perhaps we could try removing the addObserver calls for a cycle? If that doesn't work, we could also try disabling the search update code using the pref.
Comment 3•18 years ago
|
||
(In reply to comment #2)
> Perhaps we could try removing the addObserver calls for a cycle?
Sounds good to me. Does that take the usual patch/review/bake/land cycle?
Reporter | ||
Comment 4•18 years ago
|
||
If you have a debug build you can just run the leak tests yourself (assuming you are actually seeing the two leaks mentioned in the bug). See the "Running the old-style leak tests" section on http://www.mozilla.org/performance/leak-brownbag.html for how to do this.
Comment 5•18 years ago
|
||
(In reply to comment #4)
> If you have a debug build you can just run the leak tests yourself (assuming
> you are actually seeing the two leaks mentioned in the bug).
I can't get the leak test to produce consistent results, but the range of XPCWrappedNative leaks I see with the new observer enabled in search.xml is at least qualitatively the same as that produced with it commented out. It's also more like 800 bytes leaked than the 1400 mentioned. I get no nsRunnable leaks at all, and consistently 144 bytes of nsXPCWrappedJS.
I've also verified that both destructors in search.xml are being called, using the simple test of inserting a dump().
The search.xml observer may still be the culprit, but if so I can't detect it.
Comment 6•18 years ago
|
||
The 1.8.1 branch saw a similar increase in RLk between 2006-07-24 14:42 and 2006-07-24 16:00.
Checkin link:
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=AviarySuiteBranchTinderbox&branch=MOZILLA_1_8_BRANCH&date=explicit&mindate=1153777320&maxdate=1153782059
The only bug number common to both checkin lists is bug 343455, specifically mozilla/js/src/jsiter.c.
Reporter | ||
Comment 7•18 years ago
|
||
I talked to Pam about this on IRC and it looks like the leaks she mentioned in comment 6 were totally random and unrelated to the JS check-in that was made. (In fact, those leaks disappeared in the next cycle.)
mrbkap said something about the JS changes exposing more leaks, but the JS check-ins during these ranges were slightly different...
Reporter | ||
Comment 8•18 years ago
|
||
mwu has successfully tracked down the leaks:
nsRunnable - leaks randomly and is unrelated to this bug.
nsXPCWrappedJS - due to the search engine update landing (bug 327932).
XPCWrappedNative - due to bug 345350.
Comment 9•18 years ago
|
||
(In reply to comment #8)
> XPCWrappedNative - due to bug 345350.
How did you arrive at this conclusion? The patch in that bug could not possibly cause XPCWrappedNatives to leak. The only thing that that patch could do (assuming the impossible) would be to leak ScriptFilenamePrefix entries, which do not entail GC things, or even XPCOM things.
Comment 10•18 years ago
|
||
(In reply to comment #8)
> XPCWrappedNative - due to bug 345350.
See bug 345350 comment 8 (eerily similar bug number, same comment number! ;-)
Something else caused the leak. Look at surrounding hooks. Can this bug stand for the XPCWrappedNative leak, or do you want separate bugs for each leak?
/be
Reporter | ||
Comment 11•18 years ago
|
||
Filed bug 345908 for the search engine update service leak. mwu and I are still investigating the supposed leak the JS patch caused.
Comment 12•18 years ago
|
||
Sorry for the noise. I just checked again, and it appears that XPCWrappedNative is also leaked by the search engine update patch.
Comment 13•18 years ago
|
||
Sounds like this is now a dupe of bug 345908, then...
Reporter | ||
Comment 14•18 years ago
|
||
*** This bug has been marked as a duplicate of 345908 ***
You need to log in
before you can comment on or make changes to this bug.
Description
•