Last Comment Bug 626193 - mozIStorageService.backupDatabaseFile() releases database locks
: mozIStorageService.backupDatabaseFile() releases database locks
Status: NEW
:
Product: Toolkit
Classification: Components
Component: Storage (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Marco Bonardo [::mak]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-16 01:45 PST by Simon Kornblith
Modified: 2011-11-02 04:37 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Simon Kornblith 2011-01-16 01:45:51 PST
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre

When a database is open with PRAGMA locking_mode=EXCLUSIVE or with an open statement, SQLite locks the database using an fcntl-based lock. Executing mozIStorageService.backupDatabaseFile() on a locked database releases the lock, presumably because of the odd behavior of fcntl-based locks. (From the glibc documentation: "When any file descriptor for that file is closed by the process, all of the locks that process holds on that file are released, even if the locks were made using other descriptors that remain open."). Preferably, backupDatabaseFile() should try to maintain this lock. Alternatively, backupDatabaseFile() should refuse to back up a locked database.

I have only verified that this happens under OS X, but it may happen on Linux as well. Windows users are probably safe.

Reproducible: Always

Steps to Reproduce:
The following reproduces this bug from xpcshell:

// Get database file
var dbFile = Components.classes["@mozilla.org/file/directory_service;1"].
	getService(Components.interfaces.nsIProperties).
	get("TmpD", Components.interfaces.nsIFile);
dbFile.append("testdb.sqlite");
dump("Database file located at "+dbFile.path+"\n");

// Open database
var store = Components.classes["@mozilla.org/storage/service;1"].
		getService(Components.interfaces.mozIStorageService);
var connection = store.openDatabase(dbFile);

// Set exclusive lock
connection.executeSimpleSQL("PRAGMA locking_mode=EXCLUSIVE");

// Create a dummy table and start a new statement accessing it
var sql = "CREATE TABLE IF NOT EXISTS dummyTable (id INTEGER PRIMARY KEY)";
connection.executeSimpleSQL(sql);
sql = "INSERT OR IGNORE INTO dummyTable VALUES (1)";
connection.executeSimpleSQL(sql);
sql = "SELECT id FROM dummyTable LIMIT 1"
var dummyStatement = connection.createStatement(sql);
dummyStatement.executeStep();

// database is now locked, as can be seen from sqlite client

store.backupDatabaseFile(dbFile, "testdb.sqlite.bak", dbFile.parent);

// database is no longer locked
Actual Results:  
Database is unlocked by backupDatabaseFile() call

Expected Results:  
Database should remain locked following backupDatabaseFile() call
Comment 1 Eric Shepherd [:sheppy] 2011-01-18 05:38:55 PST
Simon, you added a note to docs claiming this causes database corruption, but your description in the bug report doesn't seem to indicate that that's what really happens. It sounds like it simply unlocks the database, which is not corruption per se. Can you please clarify?
Comment 2 Dan Stillman 2011-01-18 09:13:29 PST
It doesn't cause corruption per se, but it can result in corruption for programs that rely on exclusive database locking. We (Zotero) have encountered database corruption among users resulting from their being able to open a database in two clients simultaneously after a backup.
Comment 3 Eric Shepherd [:sheppy] 2011-01-18 09:18:09 PST
OK, I've clarified the warning message you added to the docs. Thanks.
Comment 4 Shawn Wilsher :sdwilsh 2011-01-18 10:31:13 PST
This API is not intended to be used with a live database.
Comment 5 Dan Stillman 2011-01-18 10:41:42 PST
OK, but the current documentation, at least, suggests otherwise: "The database should not be open, or you should ensure that no database activity is happening when you call this method." The second part implies that it can be used on a live DB, so that should be changed if it's not meant to be the case.

For the benefit of others who find this bug, we've worked around this in our code with (essentially) the following, which successfully reestablishes the lock:

Zotero.DB.query("PRAGMA locking_mode=NORMAL");
Zotero.DB.stopDummyStatement();
storageService.backupDatabaseFile(...);
Zotero.DB.query("PRAGMA locking_mode=EXCLUSIVE");
Zotero.DB.startDummyStatement();
Comment 6 Simon Kornblith 2011-01-19 22:14:45 PST
As far as I can tell, in all or almost all cases where backupDatabaseFile is used on the trunk, the database could potentially be open. It looks like none of these use locking_mode=EXCLUSIVE, so this bug probably does not affect them. In the version of nsNavHistory that shipped with Mozilla 1.9.2, the database is set with locking_mode=EXCLUSIVE when backupDatabaseFile() is called, which may  lead to a very rare race condition. (The DB is closed immediately after the backup, so I'm not sure whether an insert during the very brief time window during which the DB is open and unlocked and Mozilla still thinks it has an exclusive lock could actually cause corruption or not.)

With the current implementation of backupDatabaseFile(), my understanding is that the programmer needs to make sure that no statements or transactions are open and either run backupDatabaseFile() only on a closed DB or avoid locking_mode=EXCLUSIVE in order to avoid potential corruption. I think that this is enough to keep in mind that it would be worth checking, but I will defer to the judgment of those who know more than I do.

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