Closed Bug 345530 Opened 18 years ago Closed 18 years ago

Increased Rlk (leak) on balsa after 2006-07-20

Categories

(Firefox :: General, defect)

x86
Linux
defect
Not set
normal

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.
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.
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.
(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?
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.
(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.
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.
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...
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.
(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.
(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
Depends on: 345908
Filed bug 345908 for the search engine update service leak. mwu and I are still investigating the supposed leak the JS patch caused.
Sorry for the noise. I just checked again, and it appears that XPCWrappedNative is also leaked by the search engine update patch.
Sounds like this is now a dupe of bug 345908, then...
*** This bug has been marked as a duplicate of 345908 ***
Status: NEW → RESOLVED
Closed: 18 years ago
No longer depends on: 345908
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.