Bug 506805 - Remove locking in AsyncExecuteStatements

This removes the use of the shared mutex in AsyncExecuteStatements.  We now rely
on PR_AtomicSet and the volatile keyword.  This results in zero lock contention
between the calling thread and the background thread if cancel is ever called.
r=asuth
r=bent
sr=vlad
This commit is contained in:
Shawn Wilsher 2009-07-29 17:24:50 -07:00
parent 6be9a5aa4e
commit c2ac8daba2
6 changed files with 44 additions and 134 deletions

View File

@ -39,7 +39,7 @@
#include "nsISupports.idl"
[scriptable, uuid(fc3c5fdc-9a87-4757-b01f-4ace2670a3a0)]
[scriptable, uuid(00da7d20-3768-4398-bedc-e310c324b3f0)]
interface mozIStoragePendingStatement : nsISupports {
/**
@ -48,8 +48,6 @@ interface mozIStoragePendingStatement : nsISupports {
*
* @note For read statements (such as SELECT), you will no longer receive any
* notifications about results once cancel is called.
*
* @returns true if canceled successfully, false otherwise.
*/
boolean cancel();
void cancel();
};

View File

@ -205,8 +205,7 @@ AsyncExecuteStatements::AsyncExecuteStatements(StatementDataArray &aStatements,
, mMaxWait(TimeDuration::FromMilliseconds(MAX_MILLISECONDS_BETWEEN_RESULTS))
, mIntervalStart(TimeStamp::Now())
, mState(PENDING)
, mCancelRequested(PR_FALSE)
, mMutex(aConnection->sharedAsyncExecutionMutex)
, mCancelRequested(0)
{
(void)mStatements.SwapElements(aStatements);
NS_ASSERTION(mStatements.Length(), "We weren't given any statements!");
@ -222,9 +221,6 @@ AsyncExecuteStatements::shouldNotify()
NS_ASSERTION(onCallingThread, "runEvent not running on the calling thread!");
#endif
// We do not need to acquire mMutex here because it can only ever be written
// to on the calling thread, and the only thread that can call us is the
// calling thread, so we know that our access is serialized.
return !mCancelRequested;
}
@ -232,8 +228,6 @@ bool
AsyncExecuteStatements::bindExecuteAndProcessStatement(StatementData &aData,
bool aLastStatement)
{
mMutex.AssertNotCurrentThreadOwns();
sqlite3_stmt *stmt(aData);
BindingParamsArray *paramsArray(aData);
@ -247,10 +241,7 @@ AsyncExecuteStatements::bindExecuteAndProcessStatement(StatementData &aData,
error = (*itr)->bind(stmt);
if (error) {
// Set our error state.
{
MutexAutoLock mutex(mMutex);
mState = ERROR;
}
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
// And notify.
(void)notifyError(error);
@ -273,13 +264,6 @@ bool
AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
bool aLastStatement)
{
mMutex.AssertNotCurrentThreadOwns();
// We need to hold the mutex for statement execution so we can properly
// reflect state in case we are canceled. We release the mutex in a few areas
// in order to allow for cancelation to occur.
MutexAutoLock lockedScope(mMutex);
// Execute our statement
bool hasResults;
do {
@ -291,7 +275,7 @@ AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
// If we have been canceled, there is no point in going on...
if (mCancelRequested) {
mState = CANCELED;
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), CANCELED);
return false;
}
@ -300,16 +284,11 @@ AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
if (mCallback && hasResults &&
NS_FAILED(buildAndNotifyResults(aStatement))) {
// We had an error notifying, so we notify on error and stop processing.
mState = ERROR;
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
{
// Drop our mutex because notifyError doesn't want it held.
MutexAutoUnlock unlockedScope(mMutex);
// Notify, and stop processing statements.
(void)notifyError(mozIStorageError::ERROR,
"An error occurred while notifying about results");
}
// Notify, and stop processing statements.
(void)notifyError(mozIStorageError::ERROR,
"An error occurred while notifying about results");
return false;
}
@ -320,11 +299,9 @@ AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
checkAndLogStatementPerformance(aStatement);
#endif
// If we are done, we need to set our state accordingly while we still hold
// our mutex. We would have already returned if we were canceled or had
// an error at this point.
// If we are done, we need to set our state accordingly.
if (aLastStatement)
mState = COMPLETED;
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), COMPLETED);
return true;
}
@ -332,8 +309,6 @@ AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
bool
AsyncExecuteStatements::executeStatement(sqlite3_stmt *aStatement)
{
mMutex.AssertCurrentThreadOwns();
while (true) {
int rc = ::sqlite3_step(aStatement);
// Stop if we have no more results.
@ -346,25 +321,17 @@ AsyncExecuteStatements::executeStatement(sqlite3_stmt *aStatement)
// Some errors are not fatal, and we can handle them and continue.
if (rc == SQLITE_BUSY) {
// We do not want to hold our mutex while we yield.
MutexAutoUnlock cancelationScope(mMutex);
// Yield, and try again
(void)::PR_Sleep(PR_INTERVAL_NO_WAIT);
continue;
}
// Set an error state.
mState = ERROR;
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
{
// Drop our mutex because notifyError doesn't want it held.
MutexAutoUnlock unlockedScope(mMutex);
// And notify.
sqlite3 *db = ::sqlite3_db_handle(aStatement);
(void)notifyError(rc, ::sqlite3_errmsg(db));
}
// And notify.
sqlite3 *db = ::sqlite3_db_handle(aStatement);
(void)notifyError(rc, ::sqlite3_errmsg(db));
// Finally, indicate that we should stop processing.
return false;
@ -375,12 +342,6 @@ nsresult
AsyncExecuteStatements::buildAndNotifyResults(sqlite3_stmt *aStatement)
{
NS_ASSERTION(mCallback, "Trying to dispatch results without a callback!");
mMutex.AssertCurrentThreadOwns();
// At this point, it is safe to not hold the mutex and allow for cancelation.
// We may add an event to the calling thread, but that thread will not end
// up running when it checks back with us to see if it should run.
MutexAutoUnlock cancelationScope(mMutex);
// Build result object if we need it.
if (!mResultSet)
@ -417,7 +378,6 @@ AsyncExecuteStatements::buildAndNotifyResults(sqlite3_stmt *aStatement)
nsresult
AsyncExecuteStatements::notifyComplete()
{
mMutex.AssertNotCurrentThreadOwns();
NS_ASSERTION(mState != PENDING,
"Still in a pending state when calling Complete!");
@ -432,7 +392,7 @@ AsyncExecuteStatements::notifyComplete()
if (mState == COMPLETED) {
nsresult rv = mTransactionManager->Commit();
if (NS_FAILED(rv)) {
mState = ERROR;
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), ERROR);
(void)notifyError(mozIStorageError::ERROR,
"Transaction failed to commit");
}
@ -447,7 +407,7 @@ AsyncExecuteStatements::notifyComplete()
// Notify about completion iff we have a callback.
if (mCallback) {
nsRefPtr<CompletionNotifier> completionEvent =
new CompletionNotifier(mCallback, mState);
new CompletionNotifier(mCallback, static_cast<ExecutionState>(mState));
NS_ENSURE_TRUE(completionEvent, NS_ERROR_OUT_OF_MEMORY);
// We no longer own mCallback (the CompletionNotifier takes ownership).
@ -463,8 +423,6 @@ nsresult
AsyncExecuteStatements::notifyError(PRInt32 aErrorCode,
const char *aMessage)
{
mMutex.AssertNotCurrentThreadOwns();
if (!mCallback)
return NS_OK;
@ -477,8 +435,6 @@ AsyncExecuteStatements::notifyError(PRInt32 aErrorCode,
nsresult
AsyncExecuteStatements::notifyError(mozIStorageError *aError)
{
mMutex.AssertNotCurrentThreadOwns();
if (!mCallback)
return NS_OK;
@ -492,7 +448,6 @@ AsyncExecuteStatements::notifyError(mozIStorageError *aError)
nsresult
AsyncExecuteStatements::notifyResults()
{
mMutex.AssertNotCurrentThreadOwns();
NS_ASSERTION(mCallback, "notifyResults called without a callback!");
nsRefPtr<CallbackResultNotifier> notifier =
@ -515,7 +470,7 @@ NS_IMPL_THREADSAFE_ISUPPORTS2(
//// mozIStoragePendingStatement
NS_IMETHODIMP
AsyncExecuteStatements::Cancel(PRBool *_successful)
AsyncExecuteStatements::Cancel()
{
#ifdef DEBUG
PRBool onCallingThread = PR_FALSE;
@ -527,17 +482,11 @@ AsyncExecuteStatements::Cancel(PRBool *_successful)
// we are trying to cancel.
NS_ENSURE_FALSE(mCancelRequested, NS_ERROR_UNEXPECTED);
{
MutexAutoLock lockedScope(mMutex);
// Indicate that we want to try and cancel at the next cancelation point.
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mCancelRequested), 1);
// We need to indicate that we want to try and cancel now.
mCancelRequested = true;
// Establish if we can cancel
*_successful = (mState == PENDING);
}
// Note, it is possible for us to return false here, and end up canceling
// Note: While we are requesting to cancel here, it is possible that we will
// not actually be able to. However, we will still suppress handleResult
// events that have been dispatched to the calling thread. This is OK,
// however, because only read statements (such as SELECT) are going to be
// posting events to the calling thread that actually check if they should
@ -553,15 +502,10 @@ NS_IMETHODIMP
AsyncExecuteStatements::Run()
{
// Do not run if we have been canceled.
bool cancelRequested;
{
MutexAutoLock lockedScope(mMutex);
cancelRequested = mCancelRequested;
if (cancelRequested)
mState = CANCELED;
}
if (cancelRequested)
if (mCancelRequested) {
(void)::PR_AtomicSet(const_cast<PRInt32 *>(&mState), CANCELED);
return notifyComplete();
}
// If there is more than one statement, run it in a transaction. We assume
// that we have been given write statements since getting a batch of read

View File

@ -44,7 +44,6 @@
#include "nsTArray.h"
#include "nsAutoPtr.h"
#include "nsThreadUtils.h"
#include "mozilla/Mutex.h"
#include "mozilla/TimeStamp.h"
#include "mozIStoragePendingStatement.h"
@ -118,8 +117,6 @@ private:
* occurs, or we are canceled. If aLastStatement is true, we should set
* mState accordingly.
*
* @pre mMutex is not held
*
* @param aData
* The StatementData to bind, execute, and then process.
* @param aLastStatement
@ -134,8 +131,6 @@ private:
* Executes a given statement until completion, an error occurs, or we are
* canceled. If aLastStatement is true, we should set mState accordingly.
*
* @pre mMutex is not held
*
* @param aStatement
* The statement to execute and then process.
* @param aLastStatement
@ -149,8 +144,6 @@ private:
/**
* Executes a statement to completion, properly handling any error conditions.
*
* @pre mMutex is held
*
* @param aStatement
* The statement to execute to completion.
* @returns true if results were obtained, false otherwise.
@ -161,8 +154,6 @@ private:
* Builds a result set up with a row from a given statement. If we meet the
* right criteria, go ahead and notify about this results too.
*
* @pre mMutex is held
*
* @param aStatement
* The statement to get the row data from.
*/
@ -170,16 +161,12 @@ private:
/**
* Notifies callback about completion, and does any necessary cleanup.
*
* @pre mMutex is not held
*/
nsresult notifyComplete();
/**
* Notifies callback about an error.
*
* @pre mMutex is not held
*
* @param aErrorCode
* The error code defined in mozIStorageError for the error.
* @param aMessage
@ -192,8 +179,6 @@ private:
/**
* Notifies the callback about a result set.
*
* @pre mMutex is not held
*/
nsresult notifyResults();
@ -216,24 +201,20 @@ private:
TimeStamp mIntervalStart;
/**
* Indicates our state of execution.
* Indicates our state of execution. This is one of the values in the
* ExecutionState enum.
*
* @note This should only be set with PR_Atomic* functions.
*/
ExecutionState mState;
volatile PRInt32 mState;
/**
* Indicates if we should try to cancel at a cancelation point.
* Indicates if we should try to cancel at a cancelation point or not. This
* is evaluated as a boolean.
*
* @note This should only be set with PR_Atomic* functions.
*/
bool mCancelRequested;
/**
* This is the mutex tat protects our state from changing between threads.
* This includes the following variables:
* - mState
* - mCancelRequested is only set on the calling thread while the lock is
* held. It is always read from within the lock on the background thread,
* but not on the calling thread (see shouldNotify for why).
*/
Mutex &mMutex;
volatile PRInt32 mCancelRequested;
};
} // namespace storage

View File

@ -245,8 +245,7 @@ aggregateFunctionFinalHelper(sqlite3_context *aCtx)
//// Connection
Connection::Connection(Service *aService)
: sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
, mDBConn(nsnull)
: mDBConn(nsnull)
, mAsyncExecutionMutex(nsAutoLock::NewLock("AsyncExecutionMutex"))
, mAsyncExecutionThreadShuttingDown(PR_FALSE)
, mTransactionMutex(nsAutoLock::NewLock("TransactionMutex"))

View File

@ -43,7 +43,6 @@
#include "nsAutoPtr.h"
#include "nsCOMPtr.h"
#include "mozilla/Mutex.h"
#include "nsString.h"
#include "nsInterfaceHashtable.h"
@ -93,13 +92,6 @@ public:
*/
already_AddRefed<nsIEventTarget> getAsyncExecutionTarget();
/**
* Mutex used by asynchronous statements to protect state. The mutex is
* declared on the connection object because there is no contention between
* asynchronous statements (they are serialized on mAsyncExecutionThread).
*/
Mutex sharedAsyncExecutionMutex;
private:
~Connection();

View File

@ -373,7 +373,6 @@ function test_immediate_cancellation()
"DELETE FROM test WHERE id = ?"
);
stmt.bindInt32Parameter(0, 0);
let reason = Ci.mozIStorageStatementCallback.REASON_CANCELED;
var pendingStatement = stmt.executeAsync({
handleResult: function(aResultSet)
{
@ -389,7 +388,9 @@ function test_immediate_cancellation()
{
print("handleCompletion(" + aReason +
") for test_immediate_cancellation");
do_check_eq(reason, aReason);
// It is possible that we finished before we canceled.
do_check_true(aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED ||
aReason == Ci.mozIStorageStatementCallback.REASON_CANCELED);
// Run the next test.
run_next_test();
@ -397,10 +398,7 @@ function test_immediate_cancellation()
});
// Cancel immediately
if (!pendingStatement.cancel()) {
// It is possible that we finished before we canceled
reason = Ci.mozIStorageStatementCallback.REASON_FINISHED;
}
pendingStatement.cancel()
stmt.finalize();
}
@ -411,7 +409,6 @@ function test_double_cancellation()
"DELETE FROM test WHERE id = ?"
);
stmt.bindInt32Parameter(0, 0);
let reason = Ci.mozIStorageStatementCallback.REASON_CANCELED;
var pendingStatement = stmt.executeAsync({
handleResult: function(aResultSet)
{
@ -427,7 +424,9 @@ function test_double_cancellation()
{
print("handleCompletion(" + aReason +
") for test_double_cancellation");
do_check_eq(reason, aReason);
// It is possible that we finished before we canceled.
do_check_true(aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED ||
aReason == Ci.mozIStorageStatementCallback.REASON_CANCELED);
// Run the next test.
run_next_test();
@ -435,10 +434,7 @@ function test_double_cancellation()
});
// Cancel immediately
if (!pendingStatement.cancel()) {
// It is possible that we finished before we canceled
reason = Ci.mozIStorageStatementCallback.REASON_FINISHED;
}
pendingStatement.cancel()
// And cancel again - expect an exception
try {