Bug 1005991 - mozStorage should not use XPCVariant off the main thread. r=asuth

This commit is contained in:
Marco Bonardo 2015-03-12 17:55:56 +01:00
parent 8e744492bd
commit fbb580fcfc
17 changed files with 305 additions and 84 deletions

View File

@ -13,6 +13,12 @@
#include "nsString.h"
#include "nsTArray.h"
#define VARIANT_BASE_IID \
{ /* 78888042-0fa3-4f7a-8b19-7996f99bf1aa */ \
0x78888042, 0x0fa3, 0x4f7a, \
{ 0x8b, 0x19, 0x79, 0x96, 0xf9, 0x9b, 0xf1, 0xaa } \
}
/**
* This class is used by the storage module whenever an nsIVariant needs to be
* returned. We provide traits for the basic sqlite types to make use easier.
@ -36,11 +42,15 @@ class Variant_base : public nsIVariant
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIVARIANT
NS_DECLARE_STATIC_IID_ACCESSOR(VARIANT_BASE_IID)
protected:
virtual ~Variant_base() { }
};
NS_DEFINE_STATIC_IID_ACCESSOR(Variant_base,
VARIANT_BASE_IID)
////////////////////////////////////////////////////////////////////////////////
//// Traits
@ -366,7 +376,7 @@ public:
//// Template Implementation
template <typename DataType, bool Adopting=false>
class Variant : public Variant_base
class Variant MOZ_FINAL : public Variant_base
{
~Variant()
{

View File

@ -227,7 +227,6 @@ AsyncStatement::getParams()
AsyncStatement::~AsyncStatement()
{
destructorAsyncFinalize();
cleanupJSHelpers();
// If we are getting destroyed on the wrong thread, proxy the connection
// release to the right thread. I'm not sure why we do this.
@ -243,23 +242,6 @@ AsyncStatement::~AsyncStatement()
}
}
void
AsyncStatement::cleanupJSHelpers()
{
// We are considered dead at this point, so any wrappers for row or params
// need to lose their reference to us.
if (mStatementParamsHolder) {
nsCOMPtr<nsIXPConnectWrappedNative> wrapper =
do_QueryInterface(mStatementParamsHolder);
nsCOMPtr<mozIStorageStatementParams> iParams =
do_QueryWrappedNative(wrapper);
AsyncStatementParams *params =
static_cast<AsyncStatementParams *>(iParams.get());
params->mStatement = nullptr;
mStatementParamsHolder = nullptr;
}
}
////////////////////////////////////////////////////////////////////////////////
//// nsISupports
@ -369,7 +351,9 @@ AsyncStatement::Finalize()
mSQLString.get()));
asyncFinalize();
cleanupJSHelpers();
// Release the params holder, so it can release the reference to us.
mStatementParamsHolder = nullptr;
return NS_OK;
}

View File

@ -67,13 +67,6 @@ public:
private:
~AsyncStatement();
/**
* Clean up the references JS helpers hold to us. For cycle-avoidance reasons
* they do not hold reference-counted references to us, so it is important
* we do this.
*/
void cleanupJSHelpers();
/**
* @return a pointer to the BindingParams object to use with our Bind*
* method.
@ -95,7 +88,7 @@ private:
/**
* Caches the JS 'params' helper for this statement.
*/
nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
nsMainThreadPtrHandle<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
/**
* Have we been explicitly finalized by the user?

View File

@ -30,6 +30,7 @@ AsyncStatementJSHelper::getParams(AsyncStatement *aStatement,
JSObject *aScopeObj,
jsval *_params)
{
MOZ_ASSERT(NS_IsMainThread());
nsresult rv;
#ifdef DEBUG
@ -45,15 +46,20 @@ AsyncStatementJSHelper::getParams(AsyncStatement *aStatement,
NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
JS::RootedObject scope(aCtx, aScopeObj);
nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
nsCOMPtr<nsIXPConnect> xpc(Service::getXPConnect());
rv = xpc->WrapNative(
aCtx,
::JS_GetGlobalForObject(aCtx, scope),
params,
NS_GET_IID(mozIStorageStatementParams),
getter_AddRefs(aStatement->mStatementParamsHolder)
getter_AddRefs(holder)
);
NS_ENSURE_SUCCESS(rv, rv);
nsRefPtr<AsyncStatementParamsHolder> paramsHolder =
new AsyncStatementParamsHolder(holder);
aStatement->mStatementParamsHolder =
new nsMainThreadPtrHolder<nsIXPConnectJSObjectHolder>(paramsHolder);
}
JS::Rooted<JSObject*> obj(aCtx);
@ -112,5 +118,34 @@ AsyncStatementJSHelper::GetProperty(nsIXPConnectWrappedNative *aWrapper,
return NS_OK;
}
////////////////////////////////////////////////////////////////////////////////
//// AsyncStatementParamsHolder
NS_IMPL_ISUPPORTS(AsyncStatementParamsHolder, nsIXPConnectJSObjectHolder);
JSObject*
AsyncStatementParamsHolder::GetJSObject()
{
return mHolder->GetJSObject();
}
AsyncStatementParamsHolder::AsyncStatementParamsHolder(nsIXPConnectJSObjectHolder* aHolder)
: mHolder(aHolder)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(mHolder);
}
AsyncStatementParamsHolder::~AsyncStatementParamsHolder()
{
MOZ_ASSERT(NS_IsMainThread());
// We are considered dead at this point, so any wrappers for row or params
// need to lose their reference to the statement.
nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(mHolder);
nsCOMPtr<mozIStorageStatementParams> iObj = do_QueryWrappedNative(wrapper);
AsyncStatementParams *obj = static_cast<AsyncStatementParams *>(iObj.get());
obj->mStatement = nullptr;
}
} // namespace storage
} // namespace mozilla

View File

@ -8,6 +8,7 @@
#define mozilla_storage_mozStorageAsyncStatementJSHelper_h_
#include "nsIXPCScriptable.h"
#include "nsIXPConnect.h"
class AsyncStatement;
@ -29,6 +30,24 @@ private:
nsresult getParams(AsyncStatement *, JSContext *, JSObject *, JS::Value *);
};
/**
* Wrapper used to clean up the references JS helpers hold to the statement.
* For cycle-avoidance reasons they do not hold reference-counted references,
* so it is important we do this.
*/
class AsyncStatementParamsHolder MOZ_FINAL : public nsIXPConnectJSObjectHolder
{
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIXPCONNECTJSOBJECTHOLDER
explicit AsyncStatementParamsHolder(nsIXPConnectJSObjectHolder* aHolder);
private:
virtual ~AsyncStatementParamsHolder();
nsCOMPtr<nsIXPConnectJSObjectHolder> mHolder;
};
} // namespace storage
} // namespace mozilla

View File

@ -16,8 +16,6 @@ class mozIStorageAsyncStatement;
namespace mozilla {
namespace storage {
class AsyncStatement;
/*
* Since mozIStorageStatementParams is just a tagging interface we do not have
* an async variant.
@ -38,7 +36,7 @@ protected:
AsyncStatement *mStatement;
friend class AsyncStatement;
friend class AsyncStatementParamsHolder;
};
} // namespace storage

View File

@ -215,7 +215,7 @@ already_AddRefed<mozIStorageError>
BindingParams::bind(sqlite3_stmt *aStatement)
{
// Iterate through all of our stored data, and bind it.
for (int32_t i = 0; i < mParameters.Count(); i++) {
for (size_t i = 0; i < mParameters.Length(); i++) {
int rc = variantToSQLiteT(BindingColumnData(aStatement, i), mParameters[i]);
if (rc != SQLITE_OK) {
// We had an error while trying to bind. Now we need to create an error
@ -274,7 +274,11 @@ AsyncBindingParams::BindByName(const nsACString &aName,
{
NS_ENSURE_FALSE(mLocked, NS_ERROR_UNEXPECTED);
mNamedParameters.Put(aName, aValue);
nsRefPtr<Variant_base> variant = convertVariantToStorageVariant(aValue);
if (!variant)
return NS_ERROR_UNEXPECTED;
mNamedParameters.Put(aName, variant);
return NS_OK;
}
@ -378,8 +382,17 @@ BindingParams::BindByIndex(uint32_t aIndex,
ENSURE_INDEX_VALUE(aIndex, mParamCount);
// Store the variant for later use.
NS_ENSURE_TRUE(mParameters.ReplaceObjectAt(aValue, aIndex),
NS_ERROR_OUT_OF_MEMORY);
nsRefPtr<Variant_base> variant = convertVariantToStorageVariant(aValue);
if (!variant)
return NS_ERROR_UNEXPECTED;
if (mParameters.Length() <= aIndex) {
(void)mParameters.SetLength(aIndex);
(void)mParameters.AppendElement(variant);
}
else {
NS_ENSURE_TRUE(mParameters.ReplaceElementAt(aIndex, variant),
NS_ERROR_OUT_OF_MEMORY);
}
return NS_OK;
}
@ -391,9 +404,17 @@ AsyncBindingParams::BindByIndex(uint32_t aIndex,
// In the asynchronous case we do not know how many parameters there are to
// bind to, so we cannot check the validity of aIndex.
// Store the variant for later use.
NS_ENSURE_TRUE(mParameters.ReplaceObjectAt(aValue, aIndex),
NS_ERROR_OUT_OF_MEMORY);
nsRefPtr<Variant_base> variant = convertVariantToStorageVariant(aValue);
if (!variant)
return NS_ERROR_UNEXPECTED;
if (mParameters.Length() <= aIndex) {
mParameters.SetLength(aIndex);
mParameters.AppendElement(variant);
}
else {
NS_ENSURE_TRUE(mParameters.ReplaceElementAt(aIndex, variant),
NS_ERROR_OUT_OF_MEMORY);
}
return NS_OK;
}

View File

@ -14,6 +14,7 @@
#include "mozStorageBindingParamsArray.h"
#include "mozStorageStatement.h"
#include "mozStorageAsyncStatement.h"
#include "Variant.h"
#include "mozIStorageBindingParams.h"
#include "IStorageBindingParamsInternal.h"
@ -57,7 +58,9 @@ protected:
virtual ~BindingParams() {}
explicit BindingParams(mozIStorageBindingParamsArray *aOwningArray);
nsCOMArray<nsIVariant> mParameters;
// Note that this is managed as a sparse array, so particular caution should
// be used for out-of-bounds usage.
nsTArray<nsRefPtr<Variant_base> > mParameters;
bool mLocked;
private:

View File

@ -154,6 +154,95 @@ convertJSValToVariant(
return nullptr;
}
Variant_base *
convertVariantToStorageVariant(nsIVariant* aVariant)
{
nsRefPtr<Variant_base> variant = do_QueryObject(aVariant);
if (variant) {
// JS helpers already convert the JS representation to a Storage Variant,
// in such a case there's nothing left to do here, so just pass-through.
return variant;
}
if (!aVariant)
return new NullVariant();
uint16_t dataType;
nsresult rv = aVariant->GetDataType(&dataType);
NS_ENSURE_SUCCESS(rv, nullptr);
switch (dataType) {
case nsIDataType::VTYPE_BOOL:
case nsIDataType::VTYPE_INT8:
case nsIDataType::VTYPE_INT16:
case nsIDataType::VTYPE_INT32:
case nsIDataType::VTYPE_UINT8:
case nsIDataType::VTYPE_UINT16:
case nsIDataType::VTYPE_UINT32:
case nsIDataType::VTYPE_INT64:
case nsIDataType::VTYPE_UINT64: {
int64_t v;
rv = aVariant->GetAsInt64(&v);
NS_ENSURE_SUCCESS(rv, nullptr);
return new IntegerVariant(v);
}
case nsIDataType::VTYPE_FLOAT:
case nsIDataType::VTYPE_DOUBLE: {
double v;
rv = aVariant->GetAsDouble(&v);
NS_ENSURE_SUCCESS(rv, nullptr);
return new FloatVariant(v);
}
case nsIDataType::VTYPE_CHAR:
case nsIDataType::VTYPE_CHAR_STR:
case nsIDataType::VTYPE_STRING_SIZE_IS:
case nsIDataType::VTYPE_UTF8STRING:
case nsIDataType::VTYPE_CSTRING: {
nsCString v;
rv = aVariant->GetAsAUTF8String(v);
NS_ENSURE_SUCCESS(rv, nullptr);
return new UTF8TextVariant(v);
}
case nsIDataType::VTYPE_WCHAR:
case nsIDataType::VTYPE_DOMSTRING:
case nsIDataType::VTYPE_WCHAR_STR:
case nsIDataType::VTYPE_WSTRING_SIZE_IS:
case nsIDataType::VTYPE_ASTRING: {
nsString v;
rv = aVariant->GetAsAString(v);
NS_ENSURE_SUCCESS(rv, nullptr);
return new TextVariant(v);
}
case nsIDataType::VTYPE_ARRAY: {
uint16_t type;
nsIID iid;
uint32_t len;
void *rawArray;
// Note this copies the array data.
rv = aVariant->GetAsArray(&type, &iid, &len, &rawArray);
NS_ENSURE_SUCCESS(rv, nullptr);
if (type == nsIDataType::VTYPE_UINT8) {
std::pair<uint8_t *, int> v(static_cast<uint8_t *>(rawArray), len);
// Take ownership of the data avoiding a further copy.
return new AdoptedBlobVariant(v);
}
}
case nsIDataType::VTYPE_EMPTY:
case nsIDataType::VTYPE_EMPTY_ARRAY:
case nsIDataType::VTYPE_VOID:
return new NullVariant();
case nsIDataType::VTYPE_ID:
case nsIDataType::VTYPE_INTERFACE:
case nsIDataType::VTYPE_INTERFACE_IS:
default:
NS_WARNING("Unsupported variant type");
return nullptr;
}
return nullptr;
}
namespace {
class CallbackEvent : public nsRunnable
{

View File

@ -16,6 +16,7 @@
#include "nsError.h"
#include "nsAutoPtr.h"
#include "js/TypeDecls.h"
#include "Variant.h"
class mozIStorageCompletionCallback;
class mozIStorageBaseStatement;
@ -68,6 +69,16 @@ void checkAndLogStatementPerformance(sqlite3_stmt *aStatement);
*/
nsIVariant *convertJSValToVariant(JSContext *aCtx, JS::Value aValue);
/**
* Convert a provided nsIVariant implementation to our own thread-safe
* refcounting implementation, if needed.
*
* @param aValue
* The original nsIVariant to be converted.
* @return a thread-safe refcounting nsIVariant implementation.
*/
Variant_base *convertVariantToStorageVariant(nsIVariant *aVariant);
/**
* Obtains an event that will notify a completion callback about completion.
*

View File

@ -431,27 +431,9 @@ Statement::internalFinalize(bool aDestructing)
asyncFinalize();
}
// We are considered dead at this point, so any wrappers for row or params
// need to lose their reference to us.
if (mStatementParamsHolder) {
nsCOMPtr<nsIXPConnectWrappedNative> wrapper =
do_QueryInterface(mStatementParamsHolder);
nsCOMPtr<mozIStorageStatementParams> iParams =
do_QueryWrappedNative(wrapper);
StatementParams *params = static_cast<StatementParams *>(iParams.get());
params->mStatement = nullptr;
mStatementParamsHolder = nullptr;
}
if (mStatementRowHolder) {
nsCOMPtr<nsIXPConnectWrappedNative> wrapper =
do_QueryInterface(mStatementRowHolder);
nsCOMPtr<mozIStorageStatementRow> iRow =
do_QueryWrappedNative(wrapper);
StatementRow *row = static_cast<StatementRow *>(iRow.get());
row->mStatement = nullptr;
mStatementRowHolder = nullptr;
}
// Release the holders, so they can release the reference to us.
mStatementParamsHolder = nullptr;
mStatementRowHolder = nullptr;
return convertResultCode(srv);
}

View File

@ -96,8 +96,8 @@ private:
* The following two members are only used with the JS helper. They cache
* the row and params objects.
*/
nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
nsCOMPtr<nsIXPConnectJSObjectHolder> mStatementRowHolder;
nsMainThreadPtrHandle<nsIXPConnectJSObjectHolder> mStatementParamsHolder;
nsMainThreadPtrHandle<nsIXPConnectJSObjectHolder> mStatementRowHolder;
/**
* Internal version of finalize that allows us to tell it if it is being
@ -109,7 +109,7 @@ private:
*/
nsresult internalFinalize(bool aDestructing);
friend class StatementJSHelper;
friend class StatementJSHelper;
};
} // storage

View File

@ -84,6 +84,7 @@ StatementJSHelper::getRow(Statement *aStatement,
JSObject *aScopeObj,
jsval *_row)
{
MOZ_ASSERT(NS_IsMainThread());
nsresult rv;
#ifdef DEBUG
@ -98,15 +99,19 @@ StatementJSHelper::getRow(Statement *aStatement,
nsCOMPtr<mozIStorageStatementRow> row(new StatementRow(aStatement));
NS_ENSURE_TRUE(row, NS_ERROR_OUT_OF_MEMORY);
nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
nsCOMPtr<nsIXPConnect> xpc(Service::getXPConnect());
rv = xpc->WrapNative(
aCtx,
::JS_GetGlobalForObject(aCtx, scope),
row,
NS_GET_IID(mozIStorageStatementRow),
getter_AddRefs(aStatement->mStatementRowHolder)
getter_AddRefs(holder)
);
NS_ENSURE_SUCCESS(rv, rv);
nsRefPtr<StatementRowHolder> rowHolder = new StatementRowHolder(holder);
aStatement->mStatementRowHolder =
new nsMainThreadPtrHolder<nsIXPConnectJSObjectHolder>(rowHolder);
}
JS::Rooted<JSObject*> obj(aCtx);
@ -123,6 +128,7 @@ StatementJSHelper::getParams(Statement *aStatement,
JSObject *aScopeObj,
jsval *_params)
{
MOZ_ASSERT(NS_IsMainThread());
nsresult rv;
#ifdef DEBUG
@ -138,15 +144,20 @@ StatementJSHelper::getParams(Statement *aStatement,
new StatementParams(aStatement);
NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
nsCOMPtr<nsIXPConnect> xpc(Service::getXPConnect());
rv = xpc->WrapNative(
aCtx,
::JS_GetGlobalForObject(aCtx, scope),
params,
NS_GET_IID(mozIStorageStatementParams),
getter_AddRefs(aStatement->mStatementParamsHolder)
getter_AddRefs(holder)
);
NS_ENSURE_SUCCESS(rv, rv);
nsRefPtr<StatementParamsHolder> paramsHolder =
new StatementParamsHolder(holder);
aStatement->mStatementParamsHolder =
new nsMainThreadPtrHolder<nsIXPConnectJSObjectHolder>(paramsHolder);
}
JS::Rooted<JSObject*> obj(aCtx);
@ -230,5 +241,45 @@ StatementJSHelper::Resolve(nsIXPConnectWrappedNative *aWrapper,
return NS_OK;
}
////////////////////////////////////////////////////////////////////////////////
//// StatementJSObjectHolder
NS_IMPL_ISUPPORTS(StatementJSObjectHolder, nsIXPConnectJSObjectHolder);
JSObject*
StatementJSObjectHolder::GetJSObject()
{
return mHolder->GetJSObject();
}
StatementJSObjectHolder::StatementJSObjectHolder(nsIXPConnectJSObjectHolder* aHolder)
: mHolder(aHolder)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(mHolder);
}
StatementParamsHolder::~StatementParamsHolder()
{
MOZ_ASSERT(NS_IsMainThread());
// We are considered dead at this point, so any wrappers for row or params
// need to lose their reference to the statement.
nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(mHolder);
nsCOMPtr<mozIStorageStatementParams> iObj = do_QueryWrappedNative(wrapper);
StatementParams *obj = static_cast<StatementParams *>(iObj.get());
obj->mStatement = nullptr;
}
StatementRowHolder::~StatementRowHolder()
{
MOZ_ASSERT(NS_IsMainThread());
// We are considered dead at this point, so any wrappers for row or params
// need to lose their reference to the statement.
nsCOMPtr<nsIXPConnectWrappedNative> wrapper = do_QueryInterface(mHolder);
nsCOMPtr<mozIStorageStatementRow> iObj = do_QueryWrappedNative(wrapper);
StatementRow *obj = static_cast<StatementRow *>(iObj.get());
obj->mStatement = nullptr;
}
} // namespace storage
} // namespace mozilla

View File

@ -25,6 +25,44 @@ private:
nsresult getParams(Statement *, JSContext *, JSObject *, JS::Value *);
};
/**
* Wrappers used to clean up the references JS helpers hold to the statement.
* For cycle-avoidance reasons they do not hold reference-counted references,
* so it is important we do this.
*/
class StatementJSObjectHolder : public nsIXPConnectJSObjectHolder
{
public:
NS_DECL_ISUPPORTS
NS_DECL_NSIXPCONNECTJSOBJECTHOLDER
explicit StatementJSObjectHolder(nsIXPConnectJSObjectHolder* aHolder);
protected:
virtual ~StatementJSObjectHolder() {};
nsCOMPtr<nsIXPConnectJSObjectHolder> mHolder;
};
class StatementParamsHolder MOZ_FINAL: public StatementJSObjectHolder {
public:
explicit StatementParamsHolder(nsIXPConnectJSObjectHolder* aHolder)
: StatementJSObjectHolder(aHolder) {
}
private:
virtual ~StatementParamsHolder();
};
class StatementRowHolder MOZ_FINAL: public StatementJSObjectHolder {
public:
explicit StatementRowHolder(nsIXPConnectJSObjectHolder* aHolder)
: StatementJSObjectHolder(aHolder) {
}
private:
virtual ~StatementRowHolder();
};
} // namespace storage
} // namespace mozilla

View File

@ -16,8 +16,6 @@ class mozIStorageStatement;
namespace mozilla {
namespace storage {
class Statement;
class StatementParams MOZ_FINAL : public mozIStorageStatementParams
, public nsIXPCScriptable
{
@ -35,7 +33,8 @@ protected:
mozIStorageStatement *mStatement;
uint32_t mParamCount;
friend class Statement;
friend class StatementParamsHolder;
friend class StatementRowHolder;
};
} // namespace storage

View File

@ -14,8 +14,6 @@
namespace mozilla {
namespace storage {
class Statement;
class StatementRow MOZ_FINAL : public mozIStorageStatementRow
, public nsIXPCScriptable
{
@ -31,7 +29,7 @@ protected:
Statement *mStatement;
friend class Statement;
friend class StatementRowHolder;
};
} // namespace storage

View File

@ -715,12 +715,7 @@ function test_bind_bogus_type_by_index()
let array = stmt.newBindingParamsArray();
let bp = array.newBindingParams();
// We get an error after calling executeAsync, not when we bind.
bp.bindByIndex(0, run_test);
array.addParams(bp);
stmt.bindParameters(array);
execAsync(stmt, {error: Ci.mozIStorageError.MISMATCH});
Assert.throws(() => bp.bindByIndex(0, run_test), /NS_ERROR_UNEXPECTED/);
stmt.finalize();
run_next_test();
@ -736,12 +731,7 @@ function test_bind_bogus_type_by_name()
let array = stmt.newBindingParamsArray();
let bp = array.newBindingParams();
// We get an error after calling executeAsync, not when we bind.
bp.bindByName("blob", run_test);
array.addParams(bp);
stmt.bindParameters(array);
execAsync(stmt, {error: Ci.mozIStorageError.MISMATCH});
Assert.throws(() => bp.bindByName("blob", run_test), /NS_ERROR_UNEXPECTED/);
stmt.finalize();
run_next_test();