228233 - Mozilla freezes leaks memory setting image source (src) via JS due to nsHttpChannel never calling OnStopRequest

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Vidar Haarr (not reading bugmail)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.6b) Gecko/20031206
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.6b) Gecko/2003120608

This page causes my Mozilla to freeze for more than 8 minutes, and consume more
than ~300 megabytes of memory, before suddenly dropping to ~30-50 megabytes,
staying like that for a while, and then dropping to ~5-10 megabytes and crash.
This happens every time. It also freezes on my Linux RH9 Mozilla 1.5 installation.

The person who reported it to me on IRC said that it worked in his version of
Internet Explorer 6, although IE froze on my computer, just like Mozilla (didn't
use as much memory, though).

It works on Opera v7.23.

The page doesn't validate, but my belief is that it's a JS error of some sort.

Reproducible: Always

Steps to Reproduce:
1. Open the page in Mozilla.
2. Wait for more than 10 minutes.
Actual Results:  
During the loading, it freezed, and after about 10 minutes it crashed.

Expected Results:  
Probably show the page.. although I believe that it has severe errors.

Comment 1

[Vidar Haarr (not reading bugmail)]

This is the guy who reported it: [e-mail].
He doesn't have a bugzilla account, so I'll mail him and ask him to create one.

I should also specify that Mozilla didn't really crash, I got a "the memory
could not be 'read'" error. So I didn't get that WinXP crash report dialog.

Comment 2

[John Flatness]

Same symptoms on my Firebird installation.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031213
Firebird/0.7+

Comment 3

[brewthatistrue]

Attachment: javascript testcase

Comment 4

[brewthatistrue]

i don't think this is an evangelism bug.
i think it's a javascript bug.

Comment 5

[Vidar Haarr (not reading bugmail)]

*** Bug 233235 has been marked as a duplicate of this bug. ***

Comment 6

[Vidar Haarr (not reading bugmail)]

brew: very good work - thanks!
This is a known issue, I think - I wonder if it should be fixed by bug 13350 ? I
didn't read through it. In any case, I can't seem to find another bug - perhaps
someone else has more luck.

-> Browser/DOM 0

Comment 7

[Andrew Schultz]

Loading the testcase now hits the 5 second maximum script limit, but chews up a
good chunk of memory during that time.  Letting the script continue continues
eating memory and it doesn't get reclaimed.

I'll attach a leak log from valgrind

==> HTTP

Comment 8

[Andrew Schultz]

Attachment: testcase

var myImage=new Image();
while (1) {
  myImage.src="http://mozilla.org/images/mlogo.gif";
}

Comment 9

[Andrew Schultz]

Attachment: valgrind leak log

I ran the testcase I attached under valgrind with current trunk and got this.

Comment 10

[Andrew Schultz]

I dumped this in HTTP because of the leak log, but that doesn't seem very
logical.  I retested with FTP and it behaves the same

==> imagelib

Comment 11

[Stuart Parmenter]

So I think the right fix for this would be to add some checks in to
nsImageLoadingContent that when the URL changes if its the same don't do
anythinkg.  I'm not sure what this breaks though.  Its also not clear why we
wouldn't be bumping off each previous request as we kick off a new one.

I'll poke at this further, but sending it to layout: images hoping bz can shed
some light.

Comment 12

[Andrew Schultz]

> add some checks in to nsImageLoadingContent that when the URL changes if its the 
> same don't do anythinkg.

that won't help this, which leaks just as badly:

      var myImage=new Image();
      var i = 1;
      while (1) {
         myImage.src="http://mozilla.org/images/mlogo"+(i++)+".gif";
      }

Comment 13

[Boris Zbarsky [:bzbarsky]]

> So I think the right fix for this would be to add some checks

We have such checks in all the cases where they're possible.

> I'm not sure what this breaks though.

It breaks various pages that have no-cache images they want to animate (client
pull animations).

I can try to take a look at this when I get back using the refcount logger; if
someone gets a chance to do that before then, that would rock.

Comment 14

[Boris Zbarsky [:bzbarsky]]

OK, so the testcase is totally an HTTP bug.  The FTP memory leak is a separate
issue, and I'll file a separate bug for it.

With HTTP, here's what's going on:  We call AsyncOpen on the HTTP channel, then
immediately call Cancel().  The URL points to a redirect, so once it's been
cached AsyncOpen will get into ReadFromCache() and that will |return
AsyncCall(&nsHttpChannel::HandleAsyncRedirect);|.

Now before HandleAsyncRedirect is called, we get canceled.  In Cancel(), all of
mProxyRequest, mTransaction, and mCachePump are null, since we're waiting on the
HandleAsyncRedirect call to actually do anything.  So we just set mStatus and
mCanceled and return.

Now when HandleAsyncRedirect is called, mStatus is an error status, so we just
close the cache entry, remove ourselves from the loadgroup and return.  At no
point is nsHttpChannel::OnStopRequest called.  So we never drop the mListener
ref.  The mListener is a ProxyRequest, which holds a strong ref to the
imgRequest, which holds a strong ref to the nsHttpChannel.  So we have a
reference cycle that never gets broken, and we leak.

We could probably fix this easily in the imagelib code by dropping the mChannel
ref when we cancel the channel, but that would still leave this http bug (eg the
fact that we're never calling OnStartRequest/OnStopRequest on the listener
passed to AsyncOpen in this case).

Darin, any idea what we can do here?  I suspect the other HandleAsync* methods
might have similar issues, btw...

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

maybe Cancel should call AsyncAbort in an else {}, i.e. if it has nothing else
to call cancel on. However, I note that AsyncAbort clears much fewer references
than OnStopRequest, and I see no way that AsyncAbort calls OnStopRequest so that
can drop the refs... it just sets up the request proxy for its mListener and
calls onstartrequest/onstoprequest on that...

Comment 16

[Boris Zbarsky [:bzbarsky]]

So I managed to reproduce the ftp leak exactly once; I haven't been able to
reproduce it since... I'm going to hold off on filing a bug on it until I (or
someone else) has a way to reliably reproduce it.

Comment 17

[Darin Fisher]

-> default owner

Comment 18

[Adam Guthrie]

*** Bug 353644 has been marked as a duplicate of this bug. ***

Comment 19

[Benjamin Smedberg]

Not going to block, but we'll look for an appropriate owner.

Comment 20

[Maksim Lin]

Attachment: test case

Hi,
I think this bug has nothing to do with network code unless the network code is somehow involved in also reading local files!
Here is a simple test case that demonstrates the bug.

Steps to Reproduce:
1. Put 18 reasonbly large jpegs (I use 1365x768) in a folder along with the attached html file.
2. Load file into firefox 2.0 on windowsXP

Expected Result:
IE 6 on winXP just happily shows the images 1 by 1 with no noticable change in mem usage.
FF 1.5.0.8 on windows2k just happily shows the images 1 by 1 with no noticable change in mem usage.

Actual:
Watch the firefox 2.0.0.1 on winXP memory usage climb indefinitely until your machine starts to crawl to a halt due to swapping and you need to close the tab!

Reproducible: Always!

I'll also see if I can reproduce this on my linux box at home.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 254396 [details]
test case

s the network code is somehow involved in also reading local files!

It is.  All URL accesses in Gecko go through the networking library.  That includes file:// URLs.

That said, the leak described in comment 14 is HTTP-specific.  If you're seeing a problem with file://, please file a new bug, attach this testcase to it, and cc me, ok?

Comment 23

[Aleksey Sanin]

Bug #401159 might be a dup of this one.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Patrick, Honza, would one of you have time to look into this?  It's still an issue as far as I can tell....

Comment 27

[Honza Bambas (:mayhemer)]

Looking into this now.

Comment 28

[Honza Bambas (:mayhemer)]

Not sure what exactly was happening before we made opening cache entries asynchronous, but now it hangs/loops/leaks because the cache IO thread is posting an event that calls httpChannel->OnCacheEntryAvailable on the main thread but that event never gets fired, because the main thread is blocked by the JS execution.  So, the first nsHttpChannel hangs waiting for the cache entry, canceling it does not have any affect.

Comment 29

[Patrick McManus [:mcmanus]]

so now the "testcase" attachment causes what looks like infinite stack depth. I think we ought to fix this.

For some reason its easier to see if you turn off e10s.

41 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24642 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24643 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24644 0x00007fffe8267dce in mozilla::net::CacheEntryHandle::Release() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24645 0x00007fffe82680bf in mozilla::net::CacheEntry::InvokeAvailableCallback(mozilla::net::CacheEntry::Callback const&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24646 0x00007fffe826840b in mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24647 0x00007fffe826657e in mozilla::net::CacheEntry::InvokeCallbacks(bool) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24648 0x00007fffe82668c1 in mozilla::net::CacheEntry::InvokeCallbacks() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24649 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24650 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24651 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24652 0x00007fffe8267dce in mozilla::net::CacheEntryHandle::Release() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24653 0x00007fffe82680bf in mozilla::net::CacheEntry::InvokeAvailableCallback(mozilla::net::CacheEntry::Callback const&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24654 0x00007fffe826840b in mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24655 0x00007fffe826657e in mozilla::net::CacheEntry::InvokeCallbacks(bool) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24656 0x00007fffe82668c1 in mozilla::net::CacheEntry::InvokeCallbacks() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24657 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24658 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24659 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24660 0x00007fffe8267dce in mozilla::net::CacheEntryHandle::Release() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24661 0x00007fffe82680bf in mozilla::net::CacheEntry::InvokeAvailableCallback(mozilla::net::CacheEntry::Callback const&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24662 0x00007fffe826840b in mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24663 0x00007fffe826657e in mozilla::net::CacheEntry::InvokeCallbacks(bool) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24664 0x00007fffe82668c1 in mozilla::net::CacheEntry::InvokeCallbacks() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24665 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24666 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24667 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24668 0x00007fffe8267dce in mozilla::net::CacheEntryHandle::Release() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24669 0x00007fffe82680bf in mozilla::net::CacheEntry::InvokeAvailableCallback(mozilla::net::CacheEntry::Callback const&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24670 0x00007fffe826840b in mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24671 0x00007fffe826657e in mozilla::net::CacheEntry::InvokeCallbacks(bool) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24672 0x00007fffe82668c1 in mozilla::net::CacheEntry::InvokeCallbacks() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24673 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24674 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24675 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24676 0x00007fffe8267dce in mozilla::net::CacheEntryHandle::Release() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24677 0x00007fffe82680bf in mozilla::net::CacheEntry::InvokeAvailableCallback(mozilla::net::CacheEntry::Callback const&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24678 0x00007fffe826840b in mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24679 0x00007fffe826657e in mozilla::net::CacheEntry::InvokeCallbacks(bool) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24680 0x00007fffe82668c1 in mozilla::net::CacheEntry::InvokeCallbacks() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24681 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24682 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24683 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24684 0x00007fffe8267dce in mozilla::net::CacheEntryHandle::Release() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24685 0x00007fffe82680bf in mozilla::net::CacheEntry::InvokeAvailableCallback(mozilla::net::CacheEntry::Callback const&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24686 0x00007fffe826840b in mozilla::net::CacheEntry::InvokeCallback(mozilla::net::CacheEntry::Callback&) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24687 0x00007fffe826657e in mozilla::net::CacheEntry::InvokeCallbacks(bool) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24688 0x00007fffe82668c1 in mozilla::net::CacheEntry::InvokeCallbacks() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24689 0x00007fffe8267b07 in mozilla::net::CacheEntry::OnHandleClosed(mozilla::net::CacheEntryHandle const*) () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24690 0x00007fffe8267d4c in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so
#24691 0x00007fffe8267d6e in mozilla::net::CacheEntryHandle::~CacheEntryHandle() () from /home/mcmanus/src/mozilla2/wd/channels/firefox/nightly/firefox/libxul.so

Comment 30

[Honza Bambas (:mayhemer)]

This is definitely fixed in bug 1154124.  Are you up to date?

Comment 31

[Patrick McManus [:mcmanus]]

1154124 was merged to central the same day I did the test.. so could go either way. I'll retest.

Comment 32

[Patrick McManus [:mcmanus]]

confirmed we're good now.