Bug 1423820 - Copy temp entities after reattaching databases to a cloned storage connection. r?mak
MozReview-Commit-ID: illWRvUv3Y
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -762,23 +762,17 @@ Connection::initialize(nsIFileURL *aFile
}
nsresult
Connection::initializeInternal()
{
MOZ_ASSERT(mDBConn);
auto guard = MakeScopeExit([&]() {
- {
- MutexAutoLock lockedScope(sharedAsyncExecutionMutex);
- mConnectionClosed = true;
- }
- MOZ_ALWAYS_TRUE(::sqlite3_close(mDBConn) == SQLITE_OK);
- mDBConn = nullptr;
- sharedDBMutex.destroy();
+ initializeFailed();
});
if (mFileURL) {
const char* dbPath = ::sqlite3_db_filename(mDBConn, "main");
MOZ_ASSERT(dbPath);
const char* telemetryFilename =
::sqlite3_uri_parameter(dbPath, "telemetryFilename");
@@ -883,16 +877,28 @@ Connection::initializeOnAsyncThread(nsIF
nsCOMPtr<nsIRunnable> event =
NewRunnableMethod("Connection::shutdownAsyncThread",
this, &Connection::shutdownAsyncThread);
Unused << NS_DispatchToMainThread(event);
}
return rv;
}
+void
+Connection::initializeFailed()
+{
+ {
+ MutexAutoLock lockedScope(sharedAsyncExecutionMutex);
+ mConnectionClosed = true;
+ }
+ MOZ_ALWAYS_TRUE(::sqlite3_close(mDBConn) == SQLITE_OK);
+ mDBConn = nullptr;
+ sharedDBMutex.destroy();
+}
+
nsresult
Connection::databaseElementExists(enum DatabaseElementType aElementType,
const nsACString &aElementName,
bool *_exists)
{
if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
// When constructing the query, make sure to SELECT the correct db's sqlite_master
@@ -1521,48 +1527,19 @@ nsresult
Connection::initializeClone(Connection* aClone, bool aReadOnly)
{
nsresult rv = mFileURL ? aClone->initialize(mFileURL)
: aClone->initialize(mDatabaseFile);
if (NS_FAILED(rv)) {
return rv;
}
- // Copy over temporary tables, triggers, and views from the original
- // connections. Entities in `sqlite_temp_master` are only visible to the
- // connection that created them.
- if (!aReadOnly) {
- nsCOMPtr<mozIStorageStatement> stmt;
- rv = CreateStatement(NS_LITERAL_CSTRING("SELECT sql FROM sqlite_temp_master "
- "WHERE type IN ('table', 'view', "
- "'index', 'trigger')"),
- getter_AddRefs(stmt));
- // Propagate errors, because failing to copy triggers might cause schema
- // coherency issues when writing to the database from the cloned connection.
- NS_ENSURE_SUCCESS(rv, rv);
- bool hasResult = false;
- while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
- nsAutoCString query;
- rv = stmt->GetUTF8String(0, query);
- NS_ENSURE_SUCCESS(rv, rv);
-
- // The `CREATE` SQL statements in `sqlite_temp_master` omit the `TEMP`
- // keyword. We need to add it back, or we'll recreate temporary entities
- // as persistent ones. `sqlite_temp_master` also holds `CREATE INDEX`
- // statements, but those don't need `TEMP` keywords.
- if (StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE TABLE ")) ||
- StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE TRIGGER ")) ||
- StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE VIEW "))) {
- query.Replace(0, 6, "CREATE TEMP");
- }
-
- rv = aClone->ExecuteSimpleSQL(query);
- NS_ENSURE_SUCCESS(rv, rv);
- }
- }
+ auto guard = MakeScopeExit([&]() {
+ aClone->initializeFailed();
+ });
// Re-attach on-disk databases that were attached to the original connection.
{
nsCOMPtr<mozIStorageStatement> stmt;
rv = CreateStatement(NS_LITERAL_CSTRING("PRAGMA database_list"),
getter_AddRefs(stmt));
MOZ_ASSERT(NS_SUCCEEDED(rv));
bool hasResult = false;
@@ -1618,16 +1595,55 @@ Connection::initializeClone(Connection*
if (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
pragmaQuery.AppendLiteral(" = ");
pragmaQuery.AppendInt(stmt->AsInt32(0));
rv = aClone->ExecuteSimpleSQL(pragmaQuery);
MOZ_ASSERT(NS_SUCCEEDED(rv));
}
}
+ // Copy over temporary tables, triggers, and views from the original
+ // connections. Entities in `sqlite_temp_master` are only visible to the
+ // connection that created them.
+ if (!aReadOnly) {
+ rv = aClone->ExecuteSimpleSQL(NS_LITERAL_CSTRING("BEGIN TRANSACTION"));
+ NS_ENSURE_SUCCESS(rv, rv);
+
+ nsCOMPtr<mozIStorageStatement> stmt;
+ rv = CreateStatement(NS_LITERAL_CSTRING("SELECT sql FROM sqlite_temp_master "
+ "WHERE type IN ('table', 'view', "
+ "'index', 'trigger')"),
+ getter_AddRefs(stmt));
+ // Propagate errors, because failing to copy triggers might cause schema
+ // coherency issues when writing to the database from the cloned connection.
+ NS_ENSURE_SUCCESS(rv, rv);
+ bool hasResult = false;
+ while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
+ nsAutoCString query;
+ rv = stmt->GetUTF8String(0, query);
+ NS_ENSURE_SUCCESS(rv, rv);
+
+ // The `CREATE` SQL statements in `sqlite_temp_master` omit the `TEMP`
+ // keyword. We need to add it back, or we'll recreate temporary entities
+ // as persistent ones. `sqlite_temp_master` also holds `CREATE INDEX`
+ // statements, but those don't need `TEMP` keywords.
+ if (StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE TABLE ")) ||
+ StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE TRIGGER ")) ||
+ StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE VIEW "))) {
+ query.Replace(0, 6, "CREATE TEMP");
+ }
+
+ rv = aClone->ExecuteSimpleSQL(query);
+ NS_ENSURE_SUCCESS(rv, rv);
+ }
+
+ rv = aClone->ExecuteSimpleSQL(NS_LITERAL_CSTRING("COMMIT"));
+ NS_ENSURE_SUCCESS(rv, rv);
+ }
+
// Copy any functions that have been added to this connection.
SQLiteMutexAutoLock lockedScope(sharedDBMutex);
for (auto iter = mFunctions.Iter(); !iter.Done(); iter.Next()) {
const nsACString &key = iter.Key();
Connection::FunctionInfo data = iter.UserData();
MOZ_ASSERT(data.type == Connection::FunctionInfo::SIMPLE ||
data.type == Connection::FunctionInfo::AGGREGATE,
@@ -1646,16 +1662,17 @@ Connection::initializeClone(Connection*
static_cast<mozIStorageAggregateFunction *>(data.function.get());
rv = aClone->CreateAggregateFunction(key, data.numArgs, function);
if (NS_FAILED(rv)) {
NS_WARNING("Failed to copy aggregate function to cloned connection");
}
}
}
+ guard.release();
return NS_OK;
}
NS_IMETHODIMP
Connection::Clone(bool aReadOnly,
mozIStorageConnection **_connection)
{
MOZ_ASSERT(threadOpenedOn == NS_GetCurrentThread());
--- a/storage/mozStorageConnection.h
+++ b/storage/mozStorageConnection.h
@@ -272,16 +272,17 @@ public:
*/
bool isAsyncExecutionThreadAvailable();
nsresult initializeClone(Connection *aClone, bool aReadOnly);
private:
~Connection();
nsresult initializeInternal();
+ void initializeFailed();
/**
* Sets the database into a closed state so no further actions can be
* performed.
*
* @note mDBConn is set to nullptr in this method.
*/
nsresult setClosedState();
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -1,14 +1,24 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
// This file tests the functions of mozIStorageConnection
+function fetchAllNames(conn) {
+ let names = [];
+ let stmt = conn.createStatement(`SELECT name FROM test ORDER BY name`);
+ while (stmt.executeStep()) {
+ names.push(stmt.getUTF8String(0));
+ }
+ stmt.finalize();
+ return names;
+}
+
// Test Functions
add_task(async function test_connectionReady_open() {
// there doesn't seem to be a way for the connection to not be ready (unless
// we close it with mozIStorageConnection::Close(), but we don't for this).
// It can only fail if GetPath fails on the database file, or if we run out
// of memory trying to use an in-memory database
@@ -711,36 +721,51 @@ add_task(async function test_clone_attac
let db1 = Services.storage.openUnsharedDatabase(getTestDB());
let c = 0;
function attachDB(conn, name) {
let file = Services.dirsvc.get("ProfD", Ci.nsIFile);
file.append("test_storage_" + (++c) + ".sqlite");
let db = Services.storage.openUnsharedDatabase(file);
conn.executeSimpleSQL(`ATTACH DATABASE '${db.databaseFile.path}' AS ${name}`);
+ db.executeSimpleSQL(`CREATE TABLE test_${name}(name TEXT);`);
db.close();
}
attachDB(db1, "attached_1");
attachDB(db1, "attached_2");
+ db1.executeSimpleSQL(`
+ CREATE TEMP TRIGGER test_temp_afterinsert_trigger
+ AFTER DELETE ON test_attached_1 FOR EACH ROW
+ BEGIN
+ INSERT INTO test(name) VALUES(OLD.name);
+ END`);
// These should not throw.
let stmt = db1.createStatement("SELECT * FROM attached_1.sqlite_master");
stmt.finalize();
stmt = db1.createStatement("SELECT * FROM attached_2.sqlite_master");
stmt.finalize();
+ db1.executeSimpleSQL("INSERT INTO test_attached_1(name) VALUES('asuth')");
+ db1.executeSimpleSQL("DELETE FROM test_attached_1");
+ do_check_true(fetchAllNames(db1).includes("asuth"));
// R/W clone.
let db2 = db1.clone();
do_check_true(db2.connectionReady);
// These should not throw.
stmt = db2.createStatement("SELECT * FROM attached_1.sqlite_master");
stmt.finalize();
stmt = db2.createStatement("SELECT * FROM attached_2.sqlite_master");
stmt.finalize();
+ db2.executeSimpleSQL("INSERT INTO test_attached_1(name) VALUES('past')");
+ db2.executeSimpleSQL("DELETE FROM test_attached_1");
+ let newNames = fetchAllNames(db2);
+ do_check_true(newNames.includes("past"));
+ do_check_matches(fetchAllNames(db1), newNames);
// R/O clone.
let db3 = db1.clone(true);
do_check_true(db3.connectionReady);
// These should not throw.
stmt = db3.createStatement("SELECT * FROM attached_1.sqlite_master");
stmt.finalize();
@@ -784,22 +809,17 @@ add_task(async function test_async_clone
do_print("Fire temp trigger on read-write clone");
let deleteStmt = readWriteClone.createAsyncStatement(`
DELETE FROM test_temp`);
await executeAsync(deleteStmt);
deleteStmt.finalize();
do_print("Read from original connection");
- let names = [];
- let readStmt = db.createStatement(`SELECT name FROM test`);
- while (readStmt.executeStep()) {
- names.push(readStmt.getUTF8String(0));
- }
- readStmt.finalize();
+ let names = fetchAllNames(db);
do_check_true(names.includes("mak"));
do_check_true(names.includes("standard8"));
do_check_true(names.includes("markh"));
do_print("Create read-only clone");
let readOnlyClone = await asyncClone(db, true);
do_check_true(readOnlyClone instanceof Ci.mozIStorageAsyncConnection);