Closed Bug 15818 Opened 25 years ago Closed 25 years ago

[DOGFOOD]MLK: every ImageURLImpl leaks

Categories

(Core :: Graphics: ImageLib, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: warrensomebody, Assigned: rpotts)

References

Details

(Whiteboard: [PDT-])

Bloaty: Refcounting and Memory Bloat Statistics |<-------Name------>|<--------------References-------------->|<---------------- Objects---------------->|<------Size----->| Rem Total Mean StdDev Rem Total Mean StdDev Per-Class Rem 171 ImageURLImpl 563 563 ( 282.00 +/- 281.33) 282 282 ( 141.50 +/- 140.83) 20 5640
Status: NEW → ASSIGNED
Target Milestone: M11
Summary: [leak] every ImageURLImpl leaks → [DOGFOOD][leak] every ImageURLImpl leaks
How much? How often?
We need more info to assist in PDT+ vs. PDT- decision. How bad is this leak? Thanks!
Running apprunner/mozilla. First site is mozilla.org, sample#2, sample#0: ImageURLImpl: 20 bytes/instance, and total leak of 1100 bytes.
Summary: [DOGFOOD][leak] every ImageURLImpl leaks → [DOGFOOD]MLK: every ImageURLImpl leaks
Whiteboard: [PDT-]
This leak not big enough for dogfood. Marking [PDT-]
Assignee: pnunn → neeti
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Checked in a fix
pnunn or warren, would you like to verify that this is fixed to your satisfaction? Otherwise, I'll just rubber-stamp it verified. Thanks!
I've verified that it's not showing up in the build logs anymore -- at least not that all of them have leaked. I still see a leak of one ImageURLImpl with 2 references to it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Neeti, per warren's comments, should this bug be verified, or is there additional work to do on the occasional ImageURLImpl leaks? Thanks!
Warren, when you see a a leak of one ImageURLImpl with 2 references to it, what urls are loading? When I run mozilla, and load mozilla.org and close the application, I do not see any leaks in output produced by BloatView.
I was just looking at the bloatview output on tinderbox. It still shows a leak. It's loading www.mozilla.org.
There is a difference between the outputs produced by BloatView on linux and windows. I am seeing the leak on linux output. Windows Output: 190 ImageURLImpl 20 0 28 0 ( 4.93 +/- 4.13) 55 0 ( 9.65 +/- 8.87) Linux Output: 213 ImageURLImpl 20 20 58 1 ( 13.27 +/- 12.47) 115 1 ( 26.01 +/- 25.22) Warren, could this be a bug in BloatView, or is this a real leak on linux?
Most likely a leak on linux.
Target Milestone: M11 → M12
m12
Status: REOPENED → ASSIGNED
Blocks: 18951
Blocks: 20203
Checked in a fix for the one instance of ImageURLImpl being leaked on linux.This instance created for the icon was not being freed earlier. However there one additinal instance being leaked on both windows and linux. This started showing up on 11/29 at around 9:00 pm. This happens, when you load the mozilla.org page. The image leaking is http://www.mozilla.org/images/mozilla-banner.gif On tracing it back, it looks like the ImageConsumer holding onto the ImageURLImpl is being leaked. On yesterday's build, this ImageConsumer was being released in NS_IMETHODIMP nsHTTPResponseListener::OnStopRequest(nsIChannel* channel, nsISupports* i_pContext, nsresult i_Status, const PRUnichar* i_pMsg) { ----- // The Consumer is no longer needed... NS_IF_RELEASE(mConsumer); } This release is no longer happening in today's build. I tried putting an mConsumer = 0; in the same place in today's build, but it looks like the mConsumer is not being released because, it is held on by the nsChannelListener, which is leaking. cc'ing valeski, since the NS_IF_RELEASE(mConsumer) call was removed from nsHTTPResponseListener.cpp is causing the leak.
Assignee: neeti → rpotts
Status: ASSIGNED → NEW
the release was removed because mConsumer is now an nsCOMPtr. My checkin aggrivated another bug (this leak bug) that had been waiting in the wind for someone to encounter it. Rick Potts has some code to correct a circular dependency that should clean this up (it's the true fix). I can force a release (which is very easy) on the mConsumer nsCOMPtr if people would prefer that for now??? (though I don't actually know if this would fix the leak). Sorry Rick, but I'm reassigning to you because you have a some circular ref changes that we discussed and that should fix this. If this falls back to me because of my changes to make HTTP an event sink, please reassign.
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
I checked some code in yesterday to fix this. It released the EventSink in the transport once the request was finished. This broke the circular link between the nsHTTPChannel and the nsSocketTransport. Now the Bloat log on tinderbox says that we are *only* leaking 1 ImageURLImpl.
Rick, I think the ImageConsumer which releases the ImageURL is still leaking. That's why we still have one instance of ImageURL leaking. I checked in my fix couple of cycles before you last night, and this reduced the number of ImageURL leaks from 2 to 1. Neeti
Tracing through the tinderbox bloat logs, I find that the imageURLImpl leak was fixed on 2/02/1999 01:11 by mscott's checkin to mozilla/ netwerk/ protocol/ http/ src/nsHTTPChannel.cpp to fix all leaking channel listeners that ran through the http channel. Fixed the leak by using a nsCOMPtr. The leaking channel listener was holding onto the imageConsumer , which was holding onto theImageURLImpl. Here is the tinderbox bloat log Current file: /builds/tinderbox/Linux_2.2.5-22smp_depend/bloat-cur.log Previous file: /builds/tinderbox/Linux_2.2.5-22smp_depend/bloat-prev.log TOTAL 26364 -0.81% 13080680 -0.00% --FIXED-LEAKS------------------------------ nsStr 3800 -0.52% nsVoidArray 1392 -1.14% nsStdURL 4 -50.00% ImageURLImpl 0 -100.00% NetReaderImpl 0 -100.00% nsDocumentOpenInfo 0 -100.00% ImageNetContextImpl 0 -100.00% ImageConsumer 0 -100.00% nsChannelListener 0 -100.00% TOTAL 5196 --ALL-LEAKS-------------------------------- 189 ImageURLImpl 20 0 33 0 ( 7.62 +/- 6.74) 65 0 ( 15.33 +/- 14.47)
Status: RESOLVED → VERIFIED
Rubber-stamping as Verified/Fixed. (QA has been asked not to verify memory leak bugs due to bug overload.)
No longer blocks: 18951
No longer blocks: 20203
You need to log in before you can comment on or make changes to this bug.