Closed Bug 30385 Opened 25 years ago Closed 25 years ago

Animated gif leaks memory

Categories

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

Product:
Core
Component:
Graphics: ImageLib
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: waqar, Assigned: gagan)

References

(
URL
)

Details

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

Attachments

(2 files)

One if the animated gif that leaks memory
1.62 KB, image/gif
Details
diffs for fixing the mem leak...
4.98 KB, patch
Details | Diff | Splinter Review
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: 25 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: