Closed
Bug 662986
Opened 13 years ago
Closed 3 years ago
nsContentPrefService.js should asyncClose() its DB connection
Categories
(Toolkit :: Preferences, defect)
Tracking
()
RESOLVED
WONTFIX
mozilla6
People
(Reporter: n.nethercote, Unassigned)
Details
Any DB connection on which asynchronous transactions were executed must be closed with asyncClose(), otherwise bad things can happen (eg. bug 654573). It appears that nsContestPrefService.js fails to do this (see bug 654573 comment 61). When the relevant connection(s) is GC'd, close() is (implicitly) called on it instead of asyncClose().
Reporter | ||
Comment 1•13 years ago
|
||
mak, asuth: will async connections that aren't closed with asyncClose() cause memory leaks? sqlite3_close() is never called in that case on such a connection. If so, it would be good to get an idea of how bad this leak is. What browser action causes content-prefs.sqlite connections to be opened and closed? I expect that doing that action over and over will cause the explicit/storage/sqlite/content-prefs.sqlite/* numbers in about:memory to creep up and up...
Reporter | ||
Comment 2•13 years ago
|
||
Also, in bug 654573 comment 59, asuth said: "Connections manually maintaining transactions can hang around with active locks which could starve out other connections or cause extreme growth of the write-ahead-log." So I guess that's more potential bad behaviour here.
Reporter | ||
Comment 3•13 years ago
|
||
(In reply to comment #1) > > If so, it would be good to get an idea of how bad this leak is. Bug 662989 comment 1 suggests that an unclosed SQLite connection leaks around 180KB of memory. This looks like the place where the missing asyncClose() call should be: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/contentprefs/nsContentPrefService.js#181 It looks like this only occurs at xpcom shutdown, so it's not a big deal. Still seems worth fixing, though.
Comment 4•13 years ago
|
||
(In reply to comment #3) > It looks like this only occurs at xpcom shutdown, so it's not a big deal. > Still seems worth fixing, though. Apart that it's wrong to do shutdown at xpcom-shutdown if the service is using profile files. This shutdown stuff should be moved to profile-before-change. Regarding WAL, only Places and cookies use it right now.
Reporter | ||
Comment 5•3 years ago
|
||
10 years with no action, closing.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Comment 6•3 years ago
|
||
This is fixed anyway since now the code uses Sqlite.jsm
You need to log in
before you can comment on or make changes to this bug.
Description
•