From 74a4ee10e1d2b6251cfd9994587c827dec0046d8 Mon Sep 17 00:00:00 2001 From: Shawn Wilsher Date: Wed, 22 Jul 2009 15:18:33 -0700 Subject: [PATCH] Bug 494828 - Stop using our own mutexes and use SQLite's where possible. Part 2: Use the helper object in a few places where we had our own mutexes. r=asuth --- db/sqlite3/src/sqlite.def | 1 + storage/src/mozStorageConnection.cpp | 41 ++++++++++------------------ storage/src/mozStorageConnection.h | 21 ++++++++++++-- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/db/sqlite3/src/sqlite.def b/db/sqlite3/src/sqlite.def index 3aa5775b90d..bcaf4ed374c 100644 --- a/db/sqlite3/src/sqlite.def +++ b/db/sqlite3/src/sqlite.def @@ -83,6 +83,7 @@ EXPORTS sqlite3_create_module sqlite3_data_count sqlite3_db_handle + sqlite3_db_mutex sqlite3_declare_vtab sqlite3_enable_load_extension sqlite3_enable_shared_cache diff --git a/storage/src/mozStorageConnection.cpp b/storage/src/mozStorageConnection.cpp index 29c3e23d055..349bcc81085 100644 --- a/storage/src/mozStorageConnection.cpp +++ b/storage/src/mozStorageConnection.cpp @@ -249,10 +249,8 @@ Connection::Connection(Service *aService) , mDBConn(nsnull) , mAsyncExecutionMutex(nsAutoLock::NewLock("AsyncExecutionMutex")) , mAsyncExecutionThreadShuttingDown(PR_FALSE) -, mTransactionMutex(nsAutoLock::NewLock("TransactionMutex")) +, mDBMutex("Connection::mDBMutex") , mTransactionInProgress(PR_FALSE) -, mFunctionsMutex(nsAutoLock::NewLock("FunctionsMutex")) -, mProgressHandlerMutex(nsAutoLock::NewLock("ProgressHandlerMutex")) , mProgressHandler(nsnull) , mStorageService(aService) { @@ -263,9 +261,6 @@ Connection::~Connection() { (void)Close(); nsAutoLock::DestroyLock(mAsyncExecutionMutex); - nsAutoLock::DestroyLock(mTransactionMutex); - nsAutoLock::DestroyLock(mFunctionsMutex); - nsAutoLock::DestroyLock(mProgressHandlerMutex); } NS_IMPL_THREADSAFE_ISUPPORTS1( @@ -301,9 +296,6 @@ Connection::initialize(nsIFile *aDatabaseFile) { NS_ASSERTION (!mDBConn, "Initialize called on already opened database!"); NS_ENSURE_TRUE(mAsyncExecutionMutex, NS_ERROR_OUT_OF_MEMORY); - NS_ENSURE_TRUE(mTransactionMutex, NS_ERROR_OUT_OF_MEMORY); - NS_ENSURE_TRUE(mFunctionsMutex, NS_ERROR_OUT_OF_MEMORY); - NS_ENSURE_TRUE(mProgressHandlerMutex, NS_ERROR_OUT_OF_MEMORY); int srv; nsresult rv; @@ -326,6 +318,9 @@ Connection::initialize(nsIFile *aDatabaseFile) return convertResultCode(srv); } + // Properly wrap the database handle's mutex. + mDBMutex.initWithMutex(sqlite3_db_mutex(mDBConn)); + #ifdef PR_LOGGING if (!gStorageLog) gStorageLog = ::PR_NewLogModule("mozStorage"); @@ -444,7 +439,7 @@ Connection::databaseElementExists(enum DatabaseElementType aElementType, bool Connection::findFunctionByInstance(nsISupports *aInstance) { - PR_ASSERT_CURRENT_THREAD_OWNS_LOCK(mFunctionsMutex); + mDBMutex.assertCurrentThreadOwns(); FFEArguments args = { aInstance, false }; mFunctions.EnumerateRead(findFunctionEnumerator, &args); return args.found; @@ -460,7 +455,7 @@ Connection::sProgressHelper(void *aArg) int Connection::progressHandler() { - nsAutoLock mutex(mProgressHandlerMutex); + mDBMutex.assertCurrentThreadOwns(); if (mProgressHandler) { PRBool result; nsresult rv = mProgressHandler->OnProgress(this, &result); @@ -512,12 +507,6 @@ Connection::Close() } #endif - { - nsAutoLock mutex(mProgressHandlerMutex); - if (mProgressHandler) - ::sqlite3_progress_handler(mDBConn, 0, NULL, NULL); - } - int srv = ::sqlite3_close(mDBConn); NS_ASSERTION(srv == SQLITE_OK, "sqlite3_close failed. There are probably outstanding statements that are listed above!"); @@ -716,7 +705,7 @@ Connection::IndexExists(const nsACString &aIndexName, NS_IMETHODIMP Connection::GetTransactionInProgress(PRBool *_inProgress) { - nsAutoLock mutex(mTransactionMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); *_inProgress = mTransactionInProgress; return NS_OK; } @@ -730,7 +719,7 @@ Connection::BeginTransaction() NS_IMETHODIMP Connection::BeginTransactionAs(PRInt32 aTransactionType) { - nsAutoLock mutex(mTransactionMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); if (mTransactionInProgress) return NS_ERROR_FAILURE; nsresult rv; @@ -755,7 +744,7 @@ Connection::BeginTransactionAs(PRInt32 aTransactionType) NS_IMETHODIMP Connection::CommitTransaction() { - nsAutoLock mutex(mTransactionMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); if (!mTransactionInProgress) return NS_ERROR_FAILURE; nsresult rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("COMMIT TRANSACTION")); @@ -767,7 +756,7 @@ Connection::CommitTransaction() NS_IMETHODIMP Connection::RollbackTransaction() { - nsAutoLock mutex(mTransactionMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); if (!mTransactionInProgress) return NS_ERROR_FAILURE; nsresult rv = ExecuteSimpleSQL(NS_LITERAL_CSTRING("ROLLBACK TRANSACTION")); @@ -801,7 +790,7 @@ Connection::CreateFunction(const nsACString &aFunctionName, // Check to see if this function is already defined. We only check the name // because a function can be defined with the same body but different names. - nsAutoLock mutex(mFunctionsMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); NS_ENSURE_FALSE(mFunctions.Get(aFunctionName, NULL), NS_ERROR_FAILURE); int srv = ::sqlite3_create_function(mDBConn, @@ -829,7 +818,7 @@ Connection::CreateAggregateFunction(const nsACString &aFunctionName, if (!mDBConn) return NS_ERROR_NOT_INITIALIZED; // Check to see if this function name is already defined. - nsAutoLock mutex(mFunctionsMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); NS_ENSURE_FALSE(mFunctions.Get(aFunctionName, NULL), NS_ERROR_FAILURE); // Because aggregate functions depend on state across calls, you cannot have @@ -859,7 +848,7 @@ Connection::RemoveFunction(const nsACString &aFunctionName) { if (!mDBConn) return NS_ERROR_NOT_INITIALIZED; - nsAutoLock mutex(mFunctionsMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); NS_ENSURE_TRUE(mFunctions.Get(aFunctionName, NULL), NS_ERROR_FAILURE); int srv = ::sqlite3_create_function(mDBConn, @@ -886,7 +875,7 @@ Connection::SetProgressHandler(PRInt32 aGranularity, if (!mDBConn) return NS_ERROR_NOT_INITIALIZED; // Return previous one - nsAutoLock mutex(mProgressHandlerMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); NS_IF_ADDREF(*_oldHandler = mProgressHandler); if (!aHandler || aGranularity <= 0) { @@ -905,7 +894,7 @@ Connection::RemoveProgressHandler(mozIStorageProgressHandler **_oldHandler) if (!mDBConn) return NS_ERROR_NOT_INITIALIZED; // Return previous one - nsAutoLock mutex(mProgressHandlerMutex); + SQLiteMutexAutoLock lockedScope(mDBMutex); NS_IF_ADDREF(*_oldHandler = mProgressHandler); mProgressHandler = nsnull; diff --git a/storage/src/mozStorageConnection.h b/storage/src/mozStorageConnection.h index c47bd90905b..54a6598d1b9 100644 --- a/storage/src/mozStorageConnection.h +++ b/storage/src/mozStorageConnection.h @@ -48,6 +48,7 @@ #include "nsString.h" #include "nsInterfaceHashtable.h" #include "mozIStorageProgressHandler.h" +#include "SQLiteMutex.h" #include "mozIStorageConnection.h" #include "mozStorageService.h" @@ -158,13 +159,27 @@ private: */ PRBool mAsyncExecutionThreadShuttingDown; - PRLock *mTransactionMutex; + /** + * Wraps the mutex that SQLite gives us from sqlite3_db_mutex. + */ + SQLiteMutex mDBMutex; + + /** + * Tracks if we have a transaction in progress or not. Access protected by + * mDBMutex. + */ PRBool mTransactionInProgress; - PRLock *mFunctionsMutex; + /** + * Stores the mapping of a given function by name to its instance. Access is + * protected by mDBMutex. + */ nsInterfaceHashtable mFunctions; - PRLock *mProgressHandlerMutex; + /** + * Stores the registered progress handler for the database connection. Access + * is protected by mDBMutex. + */ nsCOMPtr mProgressHandler; // This is here for two reasons: 1) It's used to make sure that the