470221 - imap autosync leaves db's open

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect, P3)

Description

[David :Bienvenu]

Attachment: proposed fix

In some situations, IMAP autosync leaves the .msf files for imap folders open, because the folder caches db's.  This can cause large memory bloat if you have large folders, and consumes file handles, which are a limited resource, especially on some systems.

We need a more general solution for this that doesn't rely on the db/folder consumers, but for now, this keeps memory usage under control for me...

I believe autosync is generally done with the folder at the places where I've made it tell the folder to release its cached db pointer, but worst case, we'll just re-open the db.

Comment 1

[Emre Birol]

For message downloads, currently we close the folders in nsAutoSyncState::SetState method based on the following conditions;

if (!folderOpen && ! (folderFlags & nsMsgFolderFlags::Inbox))
  ownerFolder->SetMsgDatabase(nsnull);

Since it is guaranteed that we set the state of the folders to stCompletedIdle before we remove it from the queue, I don't think the leak stems from this part of the code.

On the other hand it looks like we forget to close the database explicitly when we update the folders, and my guess is this is the source of the problem.

My suggestion is to keep close in nsAutoSyncState::ProcessExistingHeaders method;

     mExistingHeadersQ.Clear();
     mProcessPointer = 0;
+    folder->SetMsgDatabase(nsnull);

but not introduce new database close operations to the download part since it contradicts with the final close, and closing the database every time we download a group of messages come with a price. If we really want to close the database of the pending folders during non-idle time, we can do that in the autosync manager when we get an idle-back signal from the idle service.

Comment 2

[David :Bienvenu]

ok, thx, I'll try just that one change and get back to you...

Comment 3

[David :Bienvenu]

Attachment: reduce changes per emre

ok, let's just try the part that Emre wants, since it should help and this bug is causing a fair amount of pain. We can deal with any remaining issues as they come up.

Comment 4

[David :Bienvenu]

fix checked in.

Comment 5

[David :Bienvenu]

this fix is not sufficient - I believe we need at least one of the other changes in my previous patch. I'm running with my patch and after a while, auto sync has all my imap folder db's open. Perhaps they're all waiting to be synced in a queue, it's not good. but Re-opening.

Comment 6

[Emre Birol]

In this case, I would suggest to close dbs of the queued folders when idle is back.

Comment 7

[Wayne Mery (:wsmwk)]

should this be blocking or wanted?

Comment 8

[David :Bienvenu]

I'm going to mark this blocking, yes.

Comment 9

[David :Bienvenu]

Attachment: let go of folder's cached db if status says there are no new headers

If we do a status on an imap folder and decide we don't need to update it, then we should let go of the cached folder db...

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 376981 [details] [diff] [review]
let go of folder's cached db if status says there are no new headers

Yep that's much tidier. r/sr=Standard8

Comment 11

[David :Bienvenu]

fix checked in.