ForceDBClosed() should go away.
Categories
(MailNews Core :: Database, task)
Tracking
(Not tracked)
People
(Reporter: benc, Unassigned)
References
Details
Current situation:
We've got an issue with too many MSF files being kept open.
Especially on windows, if you have large numbers of mail folders it can be a problem.
We've currently got a couple of approaches to this:
MsgDBCacheManager tries to keep a lid on the number of open DBs by clearing the .msgDatabase attribute on the nsIMsgFolder: folder.msgDatabase = null. The idea being that the refcount on the DB will drop to zero, it'll destruct, and it'll close any filehandle it's holding onto.
However, this doesn't take into account lots of other things which hold refcounted pointers to DBs, so nulling out .msgDatabase on the folder isn't guaranteed to close the DB or the filehandle.
So, to try and cope with this in an ad-hoc manner, there are folder.msgDatabase = null and SetMsgDatabase(nullptr) calls peppered all over the codebase.
There's also nsIMsgFolder.ForceDBClosed() which kills the database, regardless of refcount, effectively turning that DB object into a zombie.
nsMsgDatabase::ForceClosed() does make an attempt to invalidate msgDB iterators and to tell registered listeners that it's no longer valid, but there are other things that hold onto DB pointers which are then left holding a timebomb.
A couple of timebomb examples from the last couple of days:
https://phabricator.services.mozilla.com/D159145#5229735
https://bugzilla.mozilla.org/show_bug.cgi?id=1787609#c18
What to do about it
I think we need to decouple the filehandle lifetime from from the nsIMsgDatabase lifetime.
When the DB is opened, it's slurped into RAM. After that, we generally don't need the file open unless it's to write something out. And the vast majority of DB access is read-only.
So we need to open the MSF file to read it in. Behind the scenes we can keep open a small pool of active filehandles, or maybe even just close them immediately and trust the filesystem cache will reopen them quickly when we need to write something.
The point is that nsIMsgDatabase shouldn't rely on having the file open all the time.
Ideally, we could also keep the folder .msgDatabase attribute valid at all times (ie never null!). Then we can tear out all the null checks everywhere, and tear out all folder.msgDatabase=null attempts to free up filehandles.
This brings up the issue of (potentially) keeping all the DB object resident in RAM at once, and whether or not that would scale to millions of messages.
But folder .msgDatabase access is already lazy (the db is read upon first access), so there's no reason we couldn't extend that and have least-used DBs eject their data from RAM when asked, and doing a quick re-load of the MSF when called upon...
Ultimately we're planning to move to a global (or per-account) database anyway - Bug 1572000. I think sorting out the .msgDatabase/filehandle expectations here would make migrating to a global DB much easier.
(In reply to Ben Campbell from comment #0)
We've got an issue with too many MSF files being kept open.
Indeed, not only bug 1787609 but also bug 1554188 (from 2019).
We've proposed fixes for the two bugs mentioned in comment #1, see bug 1554188 comment #34.
See also bug 1800202.
Description
•