Commit b223079c authored by Victor Costan's avatar Victor Costan Committed by Commit Bot

sql: Switch SQLite to default to exclusive locking mode.

Chrome used the default SQLite configuration where databases were first
opened in the NORMAL (not exclusive) locking mode, and then databases
that opt in were switched to the EXCLUSIVE locking mode via a "PRAGMA
locking_mode" statement.

This CL switches the default. Databases are first opened in EXCLUSIVE
mode by changing the SQLITE_DEFAULT_LOCKING_MODE build configuration
macro. Databases that don't opt into exclusive mode are switched to the
NORMAL locking mode via a "PRAGMA locking_mode" statement.

This CL should not introduce any behavior changes in most cases. The
result of the "PRAGMA locking_mode" statement was previously ignored,
and is now checked. So, if the desired locking mode cannot be set,
sql::Database::Open() will now fail.

In the long run, Chrome's SQLite usage should be migrated to use the
exclusive locking mode, for the benefits outlined in
https://crbug.com/1120969. When that happens, the "PRAGMA locking_mode"
can go away.

Bug: 56559, 1120969
Change-Id: Ia9ac996fae45890bb5ca11168422e15e594087ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2460982
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: default avatarDarwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815540}
parent 6794f035
...@@ -1487,22 +1487,15 @@ bool Database::OpenInternal(const std::string& file_name, ...@@ -1487,22 +1487,15 @@ bool Database::OpenInternal(const std::string& file_name,
return false; return false;
} }
// If indicated, lock up the database before doing anything else, so // If indicated, enable shared mode ("NORMAL") on the database, so it can be
// that the following code doesn't have to deal with locking. // opened by multiple processes. This needs to happen before WAL mode is
// enabled.
// //
// Needs to happen before any other operation is performed in WAL mode so that // TODO(crbug.com/1120969): Remove support for non-exclusive mode.
// no operation relies on shared memory if exclusive locking is turned on. if (!exclusive_locking_) {
// err = ExecuteAndReturnErrorCode("PRAGMA locking_mode=NORMAL");
// TODO(shess): This code is brittle. Find the cases where code if (err != SQLITE_OK)
// doesn't request |exclusive_locking_| and audit that it does the return false;
// right thing with SQLITE_BUSY, and that it doesn't make
// assumptions about who might change things in the database.
// http://crbug.com/56559
if (exclusive_locking_) {
// TODO(shess): This should probably be a failure. Code which
// requests exclusive locking but doesn't get it is almost certain
// to be ill-tested.
ignore_result(Execute("PRAGMA locking_mode=EXCLUSIVE"));
} }
// Enable extended result codes to provide more color on I/O errors. // Enable extended result codes to provide more color on I/O errors.
......
...@@ -111,16 +111,25 @@ class COMPONENT_EXPORT(SQL) Database { ...@@ -111,16 +111,25 @@ class COMPONENT_EXPORT(SQL) Database {
// This must be called before Open() to have an effect. // This must be called before Open() to have an effect.
void want_wal_mode(bool enabled) { want_wal_mode_ = enabled; } void want_wal_mode(bool enabled) { want_wal_mode_ = enabled; }
// Call to put the database in exclusive locking mode. There is no "back to // Makes database accessible by only one process at a time.
// normal" flag because of some additional requirements sqlite puts on this
// transaction (requires another access to the DB) and because we don't
// actually need it.
// //
// Exclusive mode means that the database is not unlocked at the end of each // TODO(https://crbug.com/1120969): This should be the default mode. The
// transaction, which means there may be less time spent initializing the // "NORMAL" mode should be opt-in.
// next transaction because it doesn't have to re-aquire locks.
// //
// This must be called before Open() to have an effect. // SQLite supports a locking protocol that allows multiple processes to safely
// operate on the same database at the same time. The locking protocol is used
// on every transaction, and comes with a small performance penalty.
//
// Calling this method causes the locking protocol to be used once, when the
// database is opened. No other process will be able to access the database at
// the same time.
//
// This method must be called before Open() to have an effect.
//
// More details at https://www.sqlite.org/pragma.html#pragma_locking_mode
//
// SQLite's locking protocol is summarized at
// https://www.sqlite.org/c3ref/io_methods.html
void set_exclusive_locking() { exclusive_locking_ = true; } void set_exclusive_locking() { exclusive_locking_ = true; }
// Call to use alternative status-tracking for mmap. Usually this is tracked // Call to use alternative status-tracking for mmap. Usually this is tracked
......
...@@ -1252,6 +1252,18 @@ TEST_F(SQLDatabaseTest, GetAppropriateMmapSizeAltStatus) { ...@@ -1252,6 +1252,18 @@ TEST_F(SQLDatabaseTest, GetAppropriateMmapSizeAltStatus) {
ExecuteWithResult(&db(), "SELECT * FROM MmapStatus")); ExecuteWithResult(&db(), "SELECT * FROM MmapStatus"));
} }
TEST_F(SQLDatabaseTest, LockingModeExclusive) {
db().Close();
db().set_exclusive_locking();
ASSERT_TRUE(db().Open(db_path()));
EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA locking_mode"), "exclusive");
}
TEST_F(SQLDatabaseTest, LockingModeNormal) {
EXPECT_EQ(ExecuteWithResult(&db(), "PRAGMA locking_mode"), "normal");
}
TEST_F(SQLDatabaseTest, EnableWALMode) { TEST_F(SQLDatabaseTest, EnableWALMode) {
db().want_wal_mode(true); db().want_wal_mode(true);
#if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode #if defined(OS_FUCHSIA) // Exclusive mode needs to be enabled to enter WAL mode
......
...@@ -133,6 +133,9 @@ bool SQLiteDatabase::Open(const String& filename) { ...@@ -133,6 +133,9 @@ bool SQLiteDatabase::Open(const String& filename) {
opening_thread_ = CurrentThread(); opening_thread_ = CurrentThread();
if (!SQLiteStatement(*this, "PRAGMA locking_mode = NORMAL;").ExecuteCommand())
DLOG(ERROR) << "SQLite database could not set locking_mode to normal";
if (!SQLiteStatement(*this, "PRAGMA temp_store = MEMORY;").ExecuteCommand()) if (!SQLiteStatement(*this, "PRAGMA temp_store = MEMORY;").ExecuteCommand())
DLOG(ERROR) << "SQLite database could not set temp_store to memory"; DLOG(ERROR) << "SQLite database could not set temp_store to memory";
......
...@@ -40,6 +40,15 @@ sqlite_common_configuration_flags = [ ...@@ -40,6 +40,15 @@ sqlite_common_configuration_flags = [
# private, so our databases use stricter settings. # private, so our databases use stricter settings.
"SQLITE_DEFAULT_FILE_PERMISSIONS=0600", "SQLITE_DEFAULT_FILE_PERMISSIONS=0600",
# Databases are opened in EXCLUSIVE mode by default.
#
# NORMAL mode, where a database can be used by multiple processes
# simultaneously, can be enabled by executing "PRAGMA locking_mode=0".
#
# https://www.sqlite.org/compile.html#default_locking_mode
# https://www.sqlite.org/pragma.html#pragma_locking_mode
"SQLITE_DEFAULT_LOCKING_MODE=1",
# Needed by the SQL MemoryDumpProvider. # Needed by the SQL MemoryDumpProvider.
# #
# Setting this to 1 is needed to collect the information reported by # Setting this to 1 is needed to collect the information reported by
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment