213688 - memory leak copying multiple messages between servers to an imap folder

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[[e-mail]]

Using Build ID: 2003050714 (sorry, but I'm on dial-up so I can't upgrade)
Steps to reproduce problem:
1. Start mozilla -mail
2. Open an IMAP folder with 13270 messages
3. Select All
4. Copy to a new IMAP folder on another server

Expected memory utilization:
Copying messages does not consume memory.

Actual memory utilization (approx):
After step 1: 10MB
After step 2: 30MB
After step 4: 250MB

Additional information:
Although both servers were on my internal lan the copy took several hours.

Comment 1

[David :Bienvenu]

Neil, are you using drag+drop or the messages | copy menu? Do you have a lot of
folders on the destination server? One possibility is that all of the db's on
the dest folder are getting opened (as part of constructing the copy menu) and
that's causing memory bloat.

Comment 2

[[e-mail]]

Yes, I did use the copy menu, but the desination server is a completely new
Exchange server, so although it has the standard Outlook set of folders they are
all completely empty. Oh, except for Outlook's Welcome message :-P

Comment 3

[David :Bienvenu]

Did any of the flyout menus pop up for other servers? That would be sufficient
to open those databases.

If you repeat this operation (or a smaller one), do we leak/bloat an equivalent
additional amount of memory?

Comment 4

[[e-mail]]

I don't think any other folders opened...
I'll try another copy and this time I'll try logging memory usage with
Performance Monitor (should I figure out how :-)

Comment 5

[David :Bienvenu]

actually, you could just copy a smaller number of messages (say 1000) and using
the performance monitor, just see if memory use grows slowly, or jumps up right
as the copy starts.

Comment 6

[[e-mail]]

Attachment: Performance log

OK, so I started browser, started performance log, opened mail, tried to copy
4000 messages, which failed part way for some reason, so I copied 5610 messages
from another folder instead. Unfortunately I didn't watch the copy operation as
it takes so long, so I can't put times to actions.

Comment 7

[David :Bienvenu]

Attachment: fix bloat of opening mail dbs for acl

this patch makes it so we cache the imap acls in the folder cache, panacea.dat.
this will make it so we don't always open the imap db's of folders that appear
in the move/copy menu. This is probably not what Neil was seeing, but it's a
big bloat and performance problem for people with lots of large imap folders.

Comment 8

[David :Bienvenu]

Attachment: another fix to avoid opening db's

this patch makes it so we don't open the db's for all the special local mail
folders - it cuts down on bloat, especially if you have a big sent folder, or
trash.

Comment 9

[David :Bienvenu]

marking fixed - these fixes were checked in...

Comment 10

[David :Bienvenu]

Here's the stack trace for the memory bloat. I'm not sure what the cache entry
is - we shouldn't be putting stuff in the memory cache here...

    nsMemoryImpl::Alloc(UINT) [nsMemoryImpl.cpp:325]
    nsSegmentedBuffer::AppendNewSegment(void) [nsSegmentedBuffer.cpp:103]
    nsStorageStream::Write(char const*,UINT,UINT *) [nsStorageStream.cpp:192]
            while (remaining) {
                availableInSegment = mSegmentEnd - mWriteCursor;
                if (!availableInSegment) {
     =>             mWriteCursor = mSegmentedBuffer->AppendNewSegment();
                    if (!mWriteCursor) {
                        mSegmentEnd = 0;
                        rv = NS_ERROR_OUT_OF_MEMORY;
    nsCacheEntryDescriptor::nsOutputStreamWrapper::Write(char const*,UINT,UINT
*) [nsCacheEntryDescriptor.cpp:642]
    nsInputStreamTee::TeeSegment(char const*,UINT) [nsInputStreamTee.cpp:80]
            nsresult rv;
            PRUint32 bytesWritten = 0;
            while (count) {
     =>         rv = mSink->Write(buf + bytesWritten, count, &bytesWritten);
                if (NS_FAILED(rv)) {
                    // ok, this is not a fatal error... just drop our reference
to mSink
                    // and continue on as if nothing happened.
    nsInputStreamTee::Read(char *,UINT,UINT *) [nsInputStreamTee.cpp:142]
    nsMsgLocalMailFolder::CopyData(nsIInputStream *,int)
[nsLocalMailFolder.cpp:2383]
          PRUint32 linebreak_len = 0;
        
          rv = aIStream->Read(mCopyState->m_dataBuffer + mCopyState->m_leftOver,
     =>     aLength, &readCount);
          NS_ENSURE_SUCCESS(rv, rv);
          mCopyState->m_leftOver += readCount;
          mCopyState->m_dataBuffer[mCopyState->m_leftOver] ='\0';
    nsCopyMessageStreamListener::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT) [nsCopyMessageStreamListener.cpp:130]
    nsStreamListenerTee::OnDataAvailable(nsIRequest *,nsISupports
*,nsIInputStream *,UINT,UINT) [nsStreamListenerTee.cpp:97]
    nsOnDataAvailableEvent0::HandleEvent(void) [nsAsyncStreamListener.cpp:425]
    nsStreamListenerEvent0::HandlePLEvent(PLEvent *) [nsAsyncStreamListener.cpp:113]

Comment 11

[David :Bienvenu]

I've tracked this down a bit more.  I believe the memory cache is holding onto
the memory. I thought the memory cache limited itself to 4MB, but on my system,
about:cache shows its limit is ~21MB, and the atual amount of memory held onto
is much greater. I then thought that the fact that the segmented buffer size is
4K was causing the discrepancy, because many messages are 1 or 2k. The segmented
buffer code does try to realloc the last segment to the actual size needed, but
on windows, at least, I'm not convinced the realloc actually frees up
memory...I'll keep poking with Purify and see what I can find out for sure...

Comment 12

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

(In reply to comment #11)
> I thought the memory cache limited itself to 4MB

these days, it chooses its size depending on the available memory. see bug 105344.

Comment 13

[David :Bienvenu]

Attachment: proposed fix

duh, I was way off in the weeds. It really is a memory leak, and the fix is
simple...

Comment 14

[David :Bienvenu]

Comment on attachment 147165 [details] [diff] [review]
proposed fix

this affects whenever you copy multiple messages (local or imap) to an imap
server in a different account

Comment 15

[David :Bienvenu]

fixed on trunk and tbird .6 branch.

Comment 16

[chris hofmann]

Comment on attachment 147165 [details] [diff] [review]
proposed fix

a=chofmann for 1.7