TL;DR: if you’re familiar with any Mozilla (C++ or JS) code that opens an async database connection, please go and check that it calls asyncClose() to close the connection; not doing so can lead to crashes and/or memory leaks.
Specifically, in Firefox 6, when such a connection is destroyed (because it’s explicitly deallocated or its refcount drops to zero in C++, or it’s garbage collected in JS) it’ll currently cause sporadic crashes in about:memory or telemetry code or Test Pilot code. This is because memory reporters end up pointing to connections that have been freed, and so when they are used they end up (innocently) accessing memory they shouldn’t.
I’ve opened bug 662989 to avoid the crashes, but even once it’s implemented, I think that if you fail to call asyncClose() it will still cause a memory leak, because sqlite3_close() is never called on the connection and so SQLite won’t free up memory used by the connection. Also, connections that needlessly remain open can have active locks that can starve out other connections or cause extreme growth of the write-ahead-log.
As I write this, thanks to Marco Bonardo, I’m aware of three places in JS code that fail to call asyncClose() when they should:
- browser/components/preferences/aboutPermissions.js. Bug 654573 covers this, I’m about to land a patch to fix it.
- toolkit/components/contentprefs/nsContentPrefService.js. Bug 662986 covers this.
- email@example.com/modules/experiment_data_store.js. Bug 662985 covers this. Note that Test Pilot seems to fail to use asyncClose() when it should, and it also uses memory reporters. So it’s unclear which of these two things is responsible for the Test Pilot crashes of this sort seen so far; in other words, Test Pilot may be a culprit or an innocent bystander, or both.
These were found via a crude MXR search. I haven’t looked for C++ code that makes this mistake.
If you only do synchronous transactions over your database connection, the connection will be cleaned up properly, even if you don’t call close(), when it is deallocated or garbage collected. However, it’s better to close the connection as soon as you can so that the resources (memory, locks) are freed immediately.
See this MDC page for more details about database connections and the relevant APIs. It appears that these APIs are somewhat error-prone. As it happens, an NS_ENSURE_TRUE macro will fail when an async connection is destroyed without having been asyncClose’d first, and this causes a warning message to be printed to stderr. Unfortunately, stderr is spammed with all sorts of warnings (something that was discussed in the comments of a previous post of mine), and so this warning gets lost among the noise. Addressing this problem is something I’ll write about shortly.
Thanks to Andrew Sutherland and Marco Bonardo for helping me greatly with the bugs mentioned above. Any errors in this post are mine!
UPDATE: I did some investigation and found that about:permissions leaked about 180KB of memory on my machine every time it failed to asyncClose a DB connection. This showed up under the explicit/storage/sqlite/other reporter in about:memory.