Bug 635482 - Crash [@ mozalloc_abort(char const* const) | NS_DebugBreak_P | AbortIfOffMainThreadIfCheckFast ] (was: [@ mozalloc_abort(char const* const) | NS_DebugBreak_P]) from nsCycleCollectingAutoRefCnt::decr with mozilla::storage::BindingParams::Release

Be very explicit about what thread we release objects on.
r=asuth
a=blocking
This commit is contained in:
Shawn Wilsher 2011-02-28 11:52:31 -08:00
parent 7c890d0e47
commit be9f611466
2 changed files with 37 additions and 18 deletions

View File

@ -16,7 +16,7 @@
* The Original Code is mozilla.org code.
*
* The Initial Developer of the Original Code is
* Mozilla Corporation.
* the Mozilla Foundation.
* Portions created by the Initial Developer are Copyright (C) 2008
* the Initial Developer. All Rights Reserved.
*
@ -76,6 +76,7 @@ namespace storage {
namespace {
typedef AsyncExecuteStatements::ExecutionState ExecutionState;
typedef AsyncExecuteStatements::StatementDataArray StatementDataArray;
/**
* Notifies a callback with a result set.
@ -138,24 +139,24 @@ private:
};
/**
* Notifies the calling thread that the statement has finished executing. Keeps
* the AsyncExecuteStatements instance alive long enough so that it does not
* get destroyed on the async thread if there are no other references alive.
* Notifies the calling thread that the statement has finished executing. Takes
* ownership of the StatementData so it is released on the proper thread.
*/
class CompletionNotifier : public nsRunnable
{
public:
/**
* This takes ownership of the callback. It is released on the thread this is
* dispatched to (which should always be the calling thread).
* This takes ownership of the callback and the StatementData. They are
* released on the thread this is dispatched to (which should always be the
* calling thread).
*/
CompletionNotifier(mozIStorageStatementCallback *aCallback,
ExecutionState aReason,
AsyncExecuteStatements *aKeepAsyncAlive)
: mKeepAsyncAlive(aKeepAsyncAlive)
, mCallback(aCallback)
StatementDataArray &aStatementData)
: mCallback(aCallback)
, mReason(aReason)
{
mStatementData.SwapElements(aStatementData);
}
NS_IMETHOD Run()
@ -165,11 +166,16 @@ public:
NS_RELEASE(mCallback);
}
// The async thread could still hold onto a reference to us, so we need to
// make sure we release our reference to the StatementData now in case our
// destructor happens in a different thread.
mStatementData.Clear();
return NS_OK;
}
private:
nsRefPtr<AsyncExecuteStatements> mKeepAsyncAlive;
StatementDataArray mStatementData;
mozIStorageStatementCallback *mCallback;
ExecutionState mReason;
};
@ -448,8 +454,9 @@ AsyncExecuteStatements::notifyComplete()
// Always generate a completion notification; it is what guarantees that our
// destruction does not happen here on the async thread.
nsRefPtr<CompletionNotifier> completionEvent =
new CompletionNotifier(mCallback, mState, this);
NS_ENSURE_TRUE(completionEvent, NS_ERROR_OUT_OF_MEMORY);
new CompletionNotifier(mCallback, mState, mStatements);
NS_ASSERTION(mStatements.IsEmpty(),
"Should have given up ownership of mStatements!");
// We no longer own mCallback (the CompletionNotifier takes ownership).
mCallback = nsnull;

View File

@ -16,7 +16,7 @@
* The Original Code is mozilla.org code.
*
* The Initial Developer of the Original Code is
* Mozilla Corporation.
* the Mozilla Foundation.
* Portions created by the Initial Developer are Copyright (C) 2009
* the Initial Developer. All Rights Reserved.
*
@ -44,6 +44,7 @@
#include "nsAutoPtr.h"
#include "nsTArray.h"
#include "nsIEventTarget.h"
#include "mozStorageBindingParamsArray.h"
#include "mozIStorageBaseStatement.h"
@ -65,12 +66,14 @@ public:
, mParamsArray(aParamsArray)
, mStatementOwner(aStatementOwner)
{
NS_PRECONDITION(mStatementOwner, "Must have a statement owner!");
}
StatementData(const StatementData &aSource)
: mStatement(aSource.mStatement)
, mParamsArray(aSource.mParamsArray)
, mStatementOwner(aSource.mStatementOwner)
{
NS_PRECONDITION(mStatementOwner, "Must have a statement owner!");
}
StatementData()
{
@ -104,14 +107,23 @@ public:
/**
* NULLs out our sqlite3_stmt (it is held by the owner) after reseting it and
* clear all bindings to it. This is expected to occur on the async thread.
*
* We do not clear mParamsArray out because we only want to release
* mParamsArray on the calling thread because of XPCVariant addref/release
* thread-safety issues. The same holds for mStatementOwner which can be
* holding such a reference chain as well.
*/
inline void finalize()
{
NS_PRECONDITION(mStatementOwner, "Must have a statement owner!");
#ifdef DEBUG
{
nsCOMPtr<nsIEventTarget> asyncThread =
mStatementOwner->getOwner()->getAsyncExecutionTarget();
// It's possible that we are shutting down the async thread, and this
// method would return NULL as a result.
if (asyncThread) {
PRBool onAsyncThread;
NS_ASSERTION(NS_SUCCEEDED(asyncThread->IsOnCurrentThread(&onAsyncThread)) && onAsyncThread,
"This should only be running on the async thread!");
}
}
#endif
// In the AsyncStatement case we may never have populated mStatement if the
// AsyncExecuteStatements got canceled or a failure occurred in constructing
// the statement.