266679 - Thunderbird consumes excessive file-handles and memory

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[David A. Cobb]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041024 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041024 Firefox/1.0

This is "Child of BUG#204977>
Thunderbird keeps open every *.msf file ever uses.  As a direct result, the
memory footprint grows and grows and grows with time.  Probably not a
coincidence: responsiveness degrades with time until even a fairly large machine
(256Meg @ 1.4GHz) simply spends all its time thrashing virtual memory.

If this weren't bad enough, this also causes shutdown to take more time than the
operating system expects.  I've learned to just ignore Windoze kind offers to
murder the process for me, but it really is inconvenient to have to go away and
make a pot of coffee while waiting for a single program to shut down.  On
Windows, at least, shedding all those threads is a messy (and very memory
intensive) process.

SEE HOW IT SHOULD BEHAVE BELOW

Reproducible: Always
Steps to Reproduce:
1. Have a whole load of folders ( I subscribe to over 200 mail-lists ) -- it
doesn't seem to matter how much is in any of them.  
2. Do a lot of message filtering, so incomming mail causes several folders to be
accessed.
3.

Actual Results:  
See DETAIL above.  The memory footprint grows and grows until Thunderbird
totally dominates my machine.  Watching with ProcessExplorer, I see the list of
file-handles for the .msf files grow and grow and never decrease.

Expected Results:  
1. So far as I can see, filtering into a mailbox file does not require access to
the matching .msf file at all.  If I destroy the .msf file, incomming mail does
not recreate it -- only if I try to search or to display the folder is the .msf
used.  
IF THIS IS CORRECT, DON'T OPEN THE MSF AT ALL DURING FILTRATION.

2. The summary files are useful to speed up searches and displays.  That being
so, the Folder "object" should record what, if any, display panels are viewing
it; if it is not being currently viewed CLOSE THE FILES.

Comment 1

[David A. Cobb]

The following bugs are, perhaps, closely related:
BUG#216535: but I don't see that 'large mailbox' correllates.  Rather, number of
folders does.
BUG#266017: but, again, a large single message isn't the trigger for me.

Comment 2

[David :Bienvenu]

Are you using a pop3 server with filters? Is this bug just about the fact that
pop3 filtering leaves the target folder db open until you visit the folder and
then visit another folder? I thought we had a bug open on that...yes, I agree it
should be fixed. The reason we open the target folder db is to update the total
and unread message counts for the folder, and to keep it in sync with the folder
w.r.t. the folder timestamp, so we won't have to reparse the folder next time
you open it.

Comment 3

[David A. Cobb]

1. YES, I am using a POP server with lots of filters. 

2. Strictly speaking, the bug is not "about the fact that pop3 filtering leaves
the target folder db open until you visit the folder" -- If I put on my Software
Engineer hat, I might care about that, but with my user hat on I only care about
the direct consequences of that design -- an unacceptably large memory footprint
and degrading responsiveness.  

"Until [I] visit" one of those folders might be weeks!!  Sadly, I can't read all
of my incomming every day.  In fact, that is one of the reasons filtering is so
important to me. 

And, FWIW, the message-count could just as well be in the volatile 'db' where
the tree view ( left hand pane of 3-pane ) information is cached.  Oh, yeah! -
it is.  Consider again, please, the fact that, if the .msf summary file is
discovered broken or missing, the filtering logic finds no need to recreate it.
 Has anyone complained that this causes any problems?  Sure, it is more
"convenient" for the coders to never worry about reclaiming any resources --
memory is cheap, after all.  But it makes for lousy programs. 

The consequences of this bloat are that using Thunderbird is damned unpleasant.
 I think I have some commitment to the project or I would have chosen another
MUA long since.

Comment 4

[David :Bienvenu]

this should be fixed now - is it still happening with tbird .9 or moz 1.8a5?

Comment 5

[Wayne Mery (:wsmwk)]

Unknown. I recently switched from suite to thunderbird+firefox, and I'm finding
the performance to be somewhat degraded. IMAP, no POP.

I have some large folders (5-10k messages).  I only have 256Meg memory, so it
could be increased TB+FF memory foot print put me over the edge. Performance
under suite was always good - TB+FF not so.

Not clear to me if bug 216535 is not related.

Comment 6

[David A. Cobb]

Kudos to David <[e-mail]>!

Currently using 2004-11-21-??-trunk, this is gone and all of its unpleasant
effects are gone with it.  

I've collected a batch of supporting data -- if I can figure how to present it
I'll post it. 

Sadly, however, there is a coincident serious breakage in Filtering.  I hope the
cure to this has not been the cause.  [I'll get that bug up here today]

Comment 7

[Panagiotis Palias]

TB version 1.0.6 (20050716)
WinXP SP2 fully patched

Noticed the same problem. TB keeps getting memory from the OS until there is no 
other left. This happens when there is no connection with some of the servers 
available and TB runs. I have 4 IMAP accounts with filters and a POP without 
filters.

Comment 8

[David A. Cobb]

Seeing that this is still open, I'll at least ask the question here:

Since reporting this on a Windoze machine, I've freed myself.  I'm running
nightlies on a Linux (Debian, kernel 2.6.12) system.

And the same damn symptom is showing up there!  If I leave TBird overnight, say,
picking up my mail, by morning it has a "footprint" of around 384Meg Virtual. 
And my system is starting to thrash.  A mail download simply takes over the
whole machine, and junk processing is worse!  I am forced to shut the prog down
and restart it fairly often.

I BELIEVE THIS IS THE SAME PROBLEM (keeping every file open when not in use) but
it is not as simple to prove it.  SHOULD I OPEN A NEW BUG for the Linux issue?

(As a secondary problem, am I wasting my time since no-one seems minded to do
much about this?)

Comment 9

[David :Bienvenu]

We don't keep every single file open anymore, unless it regressed (linux and
windows should be the same in this respect, since it's all backend code). I did
a lot of work to make us not keep files open, so I'm a little unclear why you
think no one cared. Do you have any evidence that that's the problem? Linux
surely has tools to tell you what file handles are open...

Comment 10

[David A. Cobb]

Sorry, David.  As I said, it is less easy to pull the information out, but I'm
sure there's a way since tools on Linux are not in short supply (just the
opposite).  

The 384Meg footprint is, however, an actual observation.  That being the case,
woul not a separate bug be appropriate?  (David, feel free to email me directly).

Comment 11

[David :Bienvenu]

the ref-counting problem with .msf file db's seems to be back on the trunk. I'll
look into it (and see if it's on the 1.5 beta branch as well)

Comment 12

[David A. Cobb]

FWIW, I'm using 2005-09-13-13-Aviary.  I've had to stay away from the trunk on
Linux builds because of problems with extensions being incompatable.

Comment 13

[David :Bienvenu]

Attachment: fix for regression

release the db reference when there are no more elements in the iterator - the
db shouldn't be used any more after there are no more elements, and will return
an error if it is. This gets rid of a cycle between the db and the enumerator,
because the db holds onto its listeners, and the enumerator holds a ref to the
db. I thought about using a weak reference, but the enumerator needs a pointer
to the db object itself, not an nsIMsgDatabase, so I'd need to jump through
static cast hoops to make the weak ref useful.

Comment 14

[David :Bienvenu]

this regression should be fixed for beta 2.

Comment 15

[David :Bienvenu]

Attachment: don't add a ref to db in enumerator

After talking to Neil, an other fix would be to stop making the enumerator hold
a ref to the db at all. In order to do that, I had to make
NotifyAnnouncerGoingAway do its notifications before the db is torn down, so
when the thread enumerator cleans up, it still has a valid db.

Comment 16

[David :Bienvenu]

Attachment: proposed fix

Comment 17

[David :Bienvenu]

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

I'll let this bake on the trunk a little bit, but we're going to want this for
the branch if it shakes out ok on the trunk.

Comment 18

[David :Bienvenu]

fix checked into trunk.

Comment 19

[Simon Paquet [:sipaq]]

Comment on attachment 196367 [details] [diff] [review]
fix for regression

Patch is obsolete

Comment 20

[Simon Paquet [:sipaq]]

Comment on attachment 196584 [details] [diff] [review]
don't add a ref to db in enumerator

the same

Comment 21

[Mike Schroepfer]

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

Per bug meeting - approved for 1.8b5.

Comment 22

[David :Bienvenu]

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

Comment 23

[Wayne Mery (:wsmwk)]

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

Comment 24

[Wayne Mery (:wsmwk)]

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