Last Comment Bug 549794 - Too many .msf file open with many third level folders and Show only subscribed folders unchecked/disabled
: Too many .msf file open with many third level folders and Show only subscribe...
Status: RESOLVED FIXED
: fixed-seamonkey2.0.4
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 555655 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-02 18:41 PST by Boying Lu
Modified: 2010-03-29 10:35 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.4-fixed


Attachments
proposed fix with unit test (8.01 KB, patch)
2010-03-03 21:28 PST, David :Bienvenu
no flags Details | Diff | Splinter Review
proposed fix with unit test (6.60 KB, patch)
2010-03-04 11:57 PST, David :Bienvenu
neil: review+
neil: superreview+
standard8: approval‑thunderbird3.0.4+
Details | Diff | Splinter Review

Description Boying Lu 2010-03-02 18:41:21 PST
A use has many mail folders, when he launch thunderbird, he got an error which says that "too many files open". I found that user's .msf files are opened without closing.
Comment 1 David :Bienvenu 2010-03-03 21:28:41 PST
Created attachment 430230 [details] [diff] [review]
proposed fix with unit test

This should fix the issue, which was that if you have subscription turned off, verifying the existence of sub-folders caused us to leave db's open. Writing the unit test for the fix exposed several other issues, which I've fixed, but I need to verify the fixes for tomorrow.
Comment 2 Boying Lu 2010-03-04 01:10:53 PST
I've got confirm from the user, the patch does fix the bug. Thanks
Comment 3 David :Bienvenu 2010-03-04 11:57:42 PST
Created attachment 430388 [details] [diff] [review]
proposed fix with unit test

The main fix is to close the msg database after doing a list folder url. We only do this if imap subscription is turned off, so to make the unit test work, I had to fix an issue trying to turn off imap subscription - the prev attempt to move to frozen linkage continues to be the gift that keeps giving! And I noticed an unrelated issue introduced at the same time, so I fixed that as well.

The unit test creates a 4 level folder hierarchy, which causes us to ListFolders to verify existence.

I think there's a separate issue w/ escaping of folder uris in this mode, but fixing that is scary and shouldn't block this fix.
Comment 4 neil@parkwaycc.co.uk 2010-03-04 14:22:12 PST
Comment on attachment 430388 [details] [diff] [review]
proposed fix with unit test

-  if (serverKey.IsEmpty())
+  if (!serverKey.IsEmpty())
Oh dear! (I don't know about the rest of the patch but even if it doesn't belong on the branch then I would have thought that these do.)

>+            if (!m_verifiedAsOnlineFolder)
The logic might be slightly easier to read if you either a) reversed this condition and switched the blocks or b) unconditionally cleared the database, assuming this won't have a detrimental effect on removing the subfolder.
Comment 5 David :Bienvenu 2010-03-04 17:05:32 PST
fix checked in - I did unconditionally clear the database; that should be fine.

The serverKey empty stuff mostly just means we need to restart to change from subscription to non-subscription, or vice versa, afaict, so I don't think it's a huge deal.
Comment 6 David :Bienvenu 2010-03-04 17:07:49 PST
Comment on attachment 430388 [details] [diff] [review]
proposed fix with unit test

I think the fix would be fine for 3.04, but not absolutely required. imap subscription is the default, and this patch only affects the case where you're not using imap subscription. That being said, a surprising number of users/enterprises turn off subscription, from what I can tell.
Comment 7 Boying Lu 2010-03-04 19:13:49 PST
User wants to get it fixed in 3.0.4, so I suggest to fix it in 3.0.4 if possible.
Comment 8 Wayne Mery (:wsmwk, NI for questions) 2010-03-08 12:25:32 PST
(In reply to comment #6)
> (From update of attachment 430388 [details] [diff] [review])
> I think the fix would be fine for 3.04, but not absolutely required. imap
> subscription is the default, a

Might this cause big performance issues similar to what has been fixed in other cases where dbs aren't closed?

And this is "show only subscribed folders"?
I have it unchecked :)  for reasons I no longer remember.
Comment 9 David :Bienvenu 2010-03-08 12:47:06 PST
(In reply to comment #8)
> (In reply to comment #6)
> > (From update of attachment 430388 [details] [diff] [review] [details])
> > I think the fix would be fine for 3.04, but not absolutely required. imap
> > subscription is the default, a
> 
> Might this cause big performance issues similar to what has been fixed in other
> cases where dbs aren't closed?

Well, only with third level folders, and beyond, so of somewhat limited scope.
> 
> And this is "show only subscribed folders"?

This is if you don't have it checked. The default is that it should be checked...

> I have it unchecked :)  for reasons I no longer remember.

Some enterprise deployments tell there users to uncheck it, usually because they also deploy other apps that talk to the server (e.g., webmail) that don't maintain subscription.
Comment 10 David :Bienvenu 2010-03-11 07:01:40 PST
fixed for 3.0.4
Comment 11 David :Bienvenu 2010-03-29 10:35:52 PDT
*** Bug 555655 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.