Closed Bug 30385 Opened 24 years ago Closed 24 years ago

Animated gif leaks memory

Categories

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

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: waqar, Assigned: gagan)

References

()

Details

(Keywords: memory-leak, Whiteboard: [PDT+] Looks okay, but seeking verification w/Purify.)

Attachments

(2 files)

Some images on the above page leak memory.
Keywords: beta1
Blocks: 29862
I know you folks were working on this, and partial fixes.  The meg/minute leak 
rate is Very Bad. I think you folks have something that will bring it down to a 
tolerable level.  I'll PDT+ the fix (patch? hack?) that gets us out of real 
trouble, but please add info to indicate how bad the leak is if you want to go 
for further corrections.
On 3/7, we'll probably have to give up on this (unless we're still in the 
MB/minute rate :-( ).
thanks,
Jim
Whiteboard: [PDT+] w/b minus on 3/7
We leak about a meg a minute sitting on http://my.netscape.com/ as of last 
night. I filed a bug on that and it should be marked as a dup of this, but I 
can't find it in bugzilla at the moment (probably due to bugzilla being a bit 
messed up).
I was out Friday so I am seeing this for the first time now.
Do you happen to know if this is a new leak? A week ago, glowcode
gave us a ok on animateds.

I'll start looking at this with purify. If anyone else has a good
or better tool to look at leaks, I'd appreciate knowing about it/and/or
help.

-P
Status: NEW → ASSIGNED
Target Milestone: M14
http://www.cybersight.com/~bruce/mozilla-bin.20000305.log is a purify log for 
this.

A really easy way to reproduce this is to just go and visit 
http://my.netscape.com/ (what I did for that log) and then just sit there and 
watch the memory usage grow.
Same thing happens if you go to the linuxstart.com page. Just visit the page and 

let it sit there and watch the memory usage go up, about a .5MB/s.

Thanks for the log, Bruce.
-P
*** Bug 30629 has been marked as a duplicate of this bug. ***
from my NT purify it looks like the leaks are in memcache code.
Also I get an assert that there is "a failure to shut down memcache cleanly".
in mozilla/netwerk/cache/memcache/nsMemCache.cpp.

Its a start.
-P
If this is what is also in my log, it looked to me like memcache stuff was 
leaking, at least in part, due to the nsHTTPChannel being leaked in its 
entirety.

MLK: 163800 bytes leaked in 975 blocks
  * This memory was allocated from:
        malloc         [rtlib.o]
        __bUiLtIn_nEw  [libxpcom.so]
        __builtin_new  [rtlib.o]
        nsHTTPHandler::NewChannel(const 
char*,nsIURI*,nsILoadGroup*,nsIInterfaceRequestor*,unsigned int,nsIURI*,unsigned 
int,unsigned int,nsIChannel**) [nsHTTPHandler.cpp:263]
        nsIOService::NewChannelFromURI(const 
char*,nsIURI*,nsILoadGroup*,nsIInterfaceRequestor*,unsigned int,nsIURI*,unsigned 
int,unsigned int,nsIChannel**) [nsIOService.cpp:241]
        
NS_OpenURI(nsIChannel**,nsIURI*,nsILoadGroup*,nsIInterfaceRequestor*,unsigned 
int,unsigned int,unsigned int) [nsNetUtil.h:79]
        ImageNetContextImpl::GetURL(ilIURL*,NET_ReloadMethod,ilINetReader*) 
[nsImageNetContextAsync.cpp:674]
        il_image_complete(il_container_struct*) [if.cpp:1551]
        ImgDCallbk::ImgDCBHaveImageAll() [if.cpp:185]
        process_buffered_gif_input_data(gif_struct*) [gif.cpp:692]
        gif_delay_time_callback(void*) [gif.cpp:725]
        timer_callback(nsITimer*,void*) [nsImageSystemServices.cpp:70]
        nsTimerGtk::FireTimeout() [nsTimerGtk.cpp:58]
        nsTimerExpired [nsTimerGtk.cpp:175]
        g_timeout_dispatch [gmain.c:1147]
        g_main_dispatch [gmain.c:647]
        g_main_iterate [gmain.c:854]
        g_main_run     [gmain.c:912]
        gtk_main       [gtkmain.c:475]
        nsAppShell::Run() [nsAppShell.cpp:304]
        nsAppShellService::Run() [nsAppShellService.cpp:392]
        main1(int,char**,nsISplashScreen*) [nsAppRunner.cpp:769]
        main           [nsAppRunner.cpp:889]
        _start         [crt1.o]
  * Block of 168 bytes (975 times); last block at 0x292c508

Why are we leaking 975 channels?  Why are we even creating that many?  
Everything that I see coming from the memcache appears to originate with these 
nsHTTPChannel objects.
Although this is a significant leak.... I have to hope we can survice a beta1.
The 3/7 "give up" date.
Marking PDT- for beta1
PDT+ for beta2
Whiteboard: [PDT+] w/b minus on 3/7 → [PDT-] plus for beta2
I give up.
I really hope the PDT will reconsider this one.  As jar said on 3/5/00, a
"meg/minute leak is Very Bad".  So bad, in fact, that I will not be able to use
a netscape beta product that leaks like this.  I think basically any solution is
better than this leak, even Bruce's idea of disabling animated gifs.  Realize
that this affects my.netscape.com very badly.  Someone on the PDT should just
let their browser sit on my.netscape.com for an hour or so and see what happens
to their performance.  It's not pretty.

To reiterate:  I am convinced that you cannot survive a beta with this bug.
More concretely, I will not use the netscape commericial beta if it has this
problem.  I've tried to just ignore it, but it degrades performance too much to
allow me to do so.

I'm now quitting this M15 build and starting up my nice dependable m14 build.
Thanks for reconsidering.

Maybe this has something to do with using the url string vs. uri object in the 
memcache, as mentioned in bug 29853, since the image is refetched every cycle.
status:
Gagan is looking at channel code & asked me to hold off
for a day.  Any news Gagan?
-P
I would dearly love to have a fix, or a work-around to stop this leak.  If folks
can provide a safe work-around (in the extreme, I agree, disabling animated gifs
would be considered), then remove the PDT-, and re-nominate.  I think the leak
has been reduced *greatly* (and is no longer as bad as "megs/minute").
If there is new info (work-around? fix?) or evidence that this is worse than I'm
understanding, please give details, and renominate (re: remove "PDT-" to attract
attention of the PDT).
We have to ship... dates are real... so we'll have to take pain if we have
nothing to help with this in short order :-( :-(.
Please reconsider this for beta1! I was sitting on linuxstart.com, walked away
for 10 minutes, and came back to find mozilla with a 200 meg memory partition
(last night's build, 3/8/2000 midnight)!

This is *NOT* acceptable for beta1, and I don't think it has gotten better. It
will bring lesser machine to their knees before even a few pages have been
rendered. We can't afford that kind of severe, obvious bug...

Whiteboard: [PDT-] plus for beta2 → plus for beta2
I will be checking in the fix for the 29862 bug, it has been approved. This will 
reduce the memory leak quite a bit.
The JS changes that waqar is landing *should* take care of the large
(meg/minute) leak that I mentioned earlier.  Apparently JS was holding refs, and
leaking memory, for animated gifs that were no longer visible.
I expect the leak amounts to be very "tolerable."  If my information is in
error, please add info to this bug (after the landing from waqar has done its
magic).
Thanks.
waqar:
thanks for the good news...
Should this bug be reassigned to you, or should
I hold it for any other animated image related leaks?

-P
Putting on PDT- radar for beta1. 
Keywords: beta2
Whiteboard: plus for beta2 → [PDT-]plus for beta2
Pam, you should hold this bug for other animated gifs leak, 29862 does not fix 
the animated leaks, it fixes the javascript realted memory leaks.
You mean like the animated gifs leaks like the one
due to the nsHTTPChannel...... ;-)
-P
I think so, if just load an animated gif, the one I added here, you will see 
memory being leaked. linuxstart.com has lots of animated gifs, so the problem was 
compounded.
Leaks _anywhere_ are more obvious with an animated gif page.
gagan indicated to me that he had a fix for some of this (the leaking 
nsHTTPChannel objects).  However, that was somewhere around 4-5am, so it'll be a 
bit longer before he's awake.
I am awake now, and I have a fix... essentially the problem was that we were
addreffing an nsHTTPChannel (quite unnecessarily) for the cacheNetData objects.
I think this was a left over of the changes rpotts was making. Anyhow, I cleaned
that up and the nsHTTPChannel leak per animation cycle is now gone! I will check
it in soon... Bruce will have to run his magic to verify this and identify other
leaks...
diffs attached as above... need someone to r/a it...
thanks for the news, Bruce.
-p
this should be plus for beta1! cc'ing jevering per his suggestion...
BTW the diffs are from mozilla/netwerk directory and so they missed out on the
diff for imap-

Index: nsImapProtocol.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.cpp,v
retrieving revision 1.234
diff -r1.234 nsImapProtocol.cpp
6566c6566
<     rv = cacheEntry->NewChannel(m_loadGroup, this,
getter_AddRefs(cacheChannel));
---
>     rv = cacheEntry->NewChannel(m_loadGroup, getter_AddRefs(cacheChannel));
Whiteboard: [PDT-]plus for beta2
PDT, reconsider this.  I think this *needs* to be in the beta.  It is leaking
like 2 meg every 10 seconds on animated images.  This is a HUGE leak and the fix
is simple.
*** Bug 31371 has been marked as a duplicate of this bug. ***
taking over... to stomp on it with more force... other verifyable links which
cause disaster without this fix are http://www.hampsterdance.com/ and
http://www.cowdance.com/ 
Assignee: pnunn → gagan
Status: ASSIGNED → NEW
Sorry about the dup, don't know how I missed it.

The fix looks good to me.
Putting on PDT+ radar for beta1.
Whiteboard: [PDT+]
I saw gagan had a fix in hand (with several days of testing on all three
platforms)... and I even approved it for checkin (subject to chofmann's metering
etc.)  What happened to the fix? When will it land???
Whiteboard: [PDT+] → [PDT+] fix in hand (tested, reviewed, and approved)
the branch wasn't ready till when I left on Friday. I will check it in today as
soon as the tree opens (since I need to do the same with the tip)
Status: NEW → ASSIGNED
checked in on the branch, still waiting for tree open for tip...
fix checked in on tip as well... marking fixed. 
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
[Holding verification until tomorrow AM. Not currently alert enough to verify
this well.]
Using this morning's Win32 build, I'm seeing an increase in application memory
usage of 4 megabytes after leaving the www.linuxstart.com page running for 20
minutes unattended.

However, I'm not sure if this increase in memory usage is related to the
animated GIF or not, since I don't see a regular memory increase on a minute by
minute basis in the 7 minutes since I came back.

bruce@cybersight.com, might you be open to giving this a Purify check on Really
Short Notice?
Whiteboard: [PDT+] fix in hand (tested, reviewed, and approved) → [PDT+] Looks okay, but seeking verification w/Purify.
I fear asking this, but do you have purify Eli?  I know that suresh in mail/news 
QA does.
Sorry, no such fancy tools in browser QA. ;)
Order some?  I'll continue this offline though I think.
Using this morning's commercial beta-branch builds:
	* After about 1.5 hours displaying http://www.mozilla.org/quality/browser/
front-end/testcases/imaging/img-anim-stability/img-anim-stability.html, 
Netscape's memory partition increases by 200K
	* After about 30 minutes displaying the same page on Mac OS, the memory 
partition remains unchanged (rounded off at hundreds of kilobytes)
	* After about 10 minutes on Linux (assuming VSZ in ps actually means memory)

Thus, verifying as fixed. Anyone with tools to do a more detailed check on why 
the Win32 partition is increasing slightly is welcome to.
Status: RESOLVED → VERIFIED
Keywords: nsbeta2
The problem is still in the very last firefox release (but not in mozilla
suite) try http://facs.scripps.edu/surf/images/euranim.gif
REOPEN
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: