Bug 462389 - 'XPCVariant used in nsXPCException::SetThrownJSVal can cause cycle collection on non-main threads'. r=bent, sr=jst.

This commit is contained in:
Ben Newman 2008-11-05 22:42:51 -08:00
parent c031080e02
commit 6b0b398a1c
11 changed files with 92 additions and 254 deletions

View File

@ -1,164 +0,0 @@
/* -*- Mode: c++; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*- */
/* ***** BEGIN LICENSE BLOCK *****
* Version: MPL 1.1/GPL 2.0/LGPL 2.1
*
* The contents of this file are subject to the Mozilla Public License Version
* 1.1 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
* http://www.mozilla.org/MPL/
*
* Software distributed under the License is distributed on an "AS IS" basis,
* WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
* for the specific language governing rights and limitations under the
* License.
*
* The Original Code is worker threads.
*
* The Initial Developer of the Original Code is
* Mozilla Corporation.
* Portions created by the Initial Developer are Copyright (C) 2008
* the Initial Developer. All Rights Reserved.
*
* Contributor(s):
* Ben Turner <bent.mozilla@gmail.com> (Original Author)
*
* Alternatively, the contents of this file may be used under the terms of
* either the GNU General Public License Version 2 or later (the "GPL"), or
* the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
* in which case the provisions of the GPL or the LGPL are applicable instead
* of those above. If you wish to allow use of your version of this file only
* under the terms of either the GPL or the LGPL, and not to allow others to
* use your version of this file under the terms of the MPL, indicate your
* decision by deleting the provisions above and replace them with the notice
* and other provisions required by the GPL or the LGPL. If you do not delete
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
#ifndef __NSAUTOJSOBJECTHOLDER_H__
#define __NSAUTOJSOBJECTHOLDER_H__
#include "jsapi.h"
/**
* Simple class that looks and acts like a JSObject* except that it unroots
* itself automatically if Root() is ever called. Designed to be rooted on the
* context or runtime (but not both!). Also automatically nulls its JSObject*
* on Unroot and asserts that Root has been called prior to assigning an object.
*/
class nsAutoJSObjectHolder
{
public:
/**
* Default constructor, no holding.
*/
nsAutoJSObjectHolder()
: mRt(NULL), mObj(NULL), mHeld(PR_FALSE) { }
/**
* Hold by rooting on the context's runtime in the constructor, passing the
* result out.
*/
nsAutoJSObjectHolder(JSContext* aCx, JSBool* aRv = NULL,
JSObject* aObj = NULL)
: mRt(NULL), mObj(aObj), mHeld(JS_FALSE) {
JSBool rv = Hold(aCx);
if (aRv) {
*aRv = rv;
}
}
/**
* Hold by rooting on the runtime in the constructor, passing the result out.
*/
nsAutoJSObjectHolder(JSRuntime* aRt, JSBool* aRv = NULL,
JSObject* aObj = NULL)
: mRt(aRt), mObj(aObj), mHeld(JS_FALSE) {
JSBool rv = Hold(aRt);
if (aRv) {
*aRv = rv;
}
}
/**
* Always release on destruction.
*/
~nsAutoJSObjectHolder() {
Release();
}
/**
* Hold by rooting on the context's runtime.
*/
JSBool Hold(JSContext* aCx) {
return Hold(JS_GetRuntime(aCx));
}
/**
* Hold by rooting on the runtime.
*/
JSBool Hold(JSRuntime* aRt) {
if (!mHeld) {
mHeld = JS_AddNamedRootRT(aRt, &mObj, "nsAutoRootedJSObject");
if (mHeld) {
mRt = aRt;
}
}
return mHeld;
}
/**
* Manually release.
*/
void Release() {
NS_ASSERTION(!mHeld || mRt, "Bad!");
if (mHeld) {
mHeld = !JS_RemoveRootRT(mRt, &mObj);
if (!mHeld) {
mRt = NULL;
}
mObj = NULL;
}
}
/**
* Determine if Hold has been called.
*/
JSBool IsHeld() {
return mHeld;
}
/**
* Pretend to be a JSObject*.
*/
JSObject* get() const {
return mObj;
}
/**
* Pretend to be a JSObject*.
*/
operator JSObject*() const {
return get();
}
/**
* Pretend to be a JSObject*. Assert if not held.
*/
JSObject* operator=(JSObject* aOther) {
#ifdef DEBUG
if (aOther) {
NS_ASSERTION(mHeld, "Not rooted!");
}
#endif
return mObj = aOther;
}
private:
JSRuntime* mRt;
JSObject* mObj;
JSBool mHeld;
};
#endif /* __NSAUTOJSOBJECTHOLDER_H__ */

View File

@ -672,7 +672,7 @@ nsDOMWorkerScriptLoader::
ScriptCompiler::ScriptCompiler(nsDOMWorkerScriptLoader* aLoader,
const nsString& aScriptText,
const nsCString& aFilename,
nsAutoJSObjectHolder& aScriptObj)
nsAutoJSValHolder& aScriptObj)
: ScriptLoaderRunnable(aLoader),
mScriptText(aScriptText),
mFilename(aFilename),

View File

@ -50,7 +50,7 @@
// Other includes
#include "jsapi.h"
#include "nsAutoPtr.h"
#include "nsAutoJSObjectHolder.h"
#include "nsAutoJSValHolder.h"
#include "nsCOMPtr.h"
#include "nsStringGlue.h"
#include "nsTArray.h"
@ -162,12 +162,12 @@ private:
ScriptCompiler(nsDOMWorkerScriptLoader* aLoader,
const nsString& aScriptText,
const nsCString& aFilename,
nsAutoJSObjectHolder& aScriptObj);
nsAutoJSValHolder& aScriptObj);
private:
nsString mScriptText;
nsCString mFilename;
nsAutoJSObjectHolder& mScriptObj;
nsAutoJSValHolder& mScriptObj;
};
class ScriptLoaderDone : public ScriptLoaderRunnable
@ -202,7 +202,7 @@ private:
nsresult result;
nsCOMPtr<nsIURI> finalURI;
nsCOMPtr<nsIChannel> channel;
nsAutoJSObjectHolder scriptObj;
nsAutoJSValHolder scriptObj;
};
nsIThread* mTarget;

View File

@ -48,6 +48,7 @@ MODULE = xpconnect
EXPORTS = \
nsAXPCNativeCallContext.h \
xpc_map_end.h \
nsAutoJSValHolder.h \
$(NULL)
include $(topsrcdir)/config/rules.mk

View File

@ -21,6 +21,7 @@
*
* Contributor(s):
* Ben Turner <bent.mozilla@gmail.com> (Original Author)
* Ben Newman <b{enjamn,newman}@mozilla.com>
*
* Alternatively, the contents of this file may be used under the terms of
* either the GNU General Public License Version 2 or later (the "GPL"), or
@ -36,55 +37,33 @@
*
* ***** END LICENSE BLOCK ***** */
#ifndef __NSAUTOJSOBJECTHOLDER_H__
#define __NSAUTOJSOBJECTHOLDER_H__
#ifndef __NSAUTOJSVALHOLDER_H__
#define __NSAUTOJSVALHOLDER_H__
#include "jsapi.h"
/**
* Simple class that looks and acts like a JSObject* except that it unroots
* Simple class that looks and acts like a jsval except that it unroots
* itself automatically if Root() is ever called. Designed to be rooted on the
* context or runtime (but not both!). Also automatically nulls its JSObject*
* on Unroot and asserts that Root has been called prior to assigning an object.
* context or runtime (but not both!).
*/
class nsAutoJSObjectHolder
class nsAutoJSValHolder
{
public:
/**
* Default constructor, no holding.
*/
nsAutoJSObjectHolder()
: mRt(NULL), mObj(NULL), mHeld(PR_FALSE) { }
/**
* Hold by rooting on the context's runtime in the constructor, passing the
* result out.
*/
nsAutoJSObjectHolder(JSContext* aCx, JSBool* aRv = NULL,
JSObject* aObj = NULL)
: mRt(NULL), mObj(aObj), mHeld(JS_FALSE) {
JSBool rv = Hold(aCx);
if (aRv) {
*aRv = rv;
}
}
/**
* Hold by rooting on the runtime in the constructor, passing the result out.
*/
nsAutoJSObjectHolder(JSRuntime* aRt, JSBool* aRv = NULL,
JSObject* aObj = NULL)
: mRt(aRt), mObj(aObj), mHeld(JS_FALSE) {
JSBool rv = Hold(aRt);
if (aRv) {
*aRv = rv;
}
nsAutoJSValHolder()
: mRt(NULL)
, mVal(JSVAL_NULL)
, mGCThing(NULL)
, mHeld(JS_FALSE)
{
// nothing to do
}
/**
* Always release on destruction.
*/
~nsAutoJSObjectHolder() {
virtual ~nsAutoJSValHolder() {
Release();
}
@ -97,29 +76,39 @@ public:
/**
* Hold by rooting on the runtime.
* Note that mGCThing may be JSVAL_NULL, which is not a problem.
*/
JSBool Hold(JSRuntime* aRt) {
if (!mHeld) {
mHeld = JS_AddNamedRootRT(aRt, &mObj, "nsAutoRootedJSObject");
if (mHeld) {
if (JS_AddNamedRootRT(aRt, &mGCThing, "nsAutoJSValHolder")) {
mRt = aRt;
mHeld = JS_TRUE;
} else {
Release(); // out of memory
}
}
return mHeld;
}
/**
* Manually release.
* Manually release, nullifying mVal, mGCThing, and mRt, but returning
* the original jsval.
*/
void Release() {
jsval Release() {
NS_ASSERTION(!mHeld || mRt, "Bad!");
jsval oldval = mVal;
if (mHeld) {
mHeld = !JS_RemoveRootRT(mRt, &mObj);
if (!mHeld) {
mRt = NULL;
}
mObj = NULL;
JS_RemoveRootRT(mRt, &mGCThing); // infallible
mHeld = JS_FALSE;
}
mVal = JSVAL_NULL;
mGCThing = NULL;
mRt = NULL;
return oldval;
}
/**
@ -129,36 +118,47 @@ public:
return mHeld;
}
/**
* Pretend to be a JSObject*.
*/
JSObject* get() const {
return mObj;
}
/**
* Pretend to be a JSObject*.
*/
operator JSObject*() const {
return get();
return JSVAL_IS_OBJECT(mVal)
? JSVAL_TO_OBJECT(mVal)
: JSVAL_NULL;
}
/**
* Pretend to be a JSObject*. Assert if not held.
* Pretend to be a jsval.
*/
JSObject* operator=(JSObject* aOther) {
operator jsval() const { return mVal; }
nsAutoJSValHolder &operator=(JSObject* aOther) {
#ifdef DEBUG
if (aOther) {
NS_ASSERTION(mHeld, "Not rooted!");
}
#endif
return mObj = aOther;
return *this = OBJECT_TO_JSVAL(aOther);
}
nsAutoJSValHolder &operator=(jsval aOther) {
#ifdef DEBUG
if (aOther) {
NS_ASSERTION(mHeld, "Not rooted!");
}
#endif
mVal = aOther;
mGCThing = JSVAL_IS_GCTHING(aOther)
? JSVAL_TO_GCTHING(aOther)
: NULL;
return *this;
}
private:
JSRuntime* mRt;
JSObject* mObj;
jsval mVal;
void* mGCThing;
JSBool mHeld;
};
#endif /* __NSAUTOJSOBJECTHOLDER_H__ */
#endif /* __NSAUTOJSVALHOLDER_H__ */

View File

@ -465,7 +465,7 @@ pre_call_clean_up:
nsCOMPtr<nsIException> e;
XPCConvert::ConstructException(code, sz, "IDispatch", name.get(),
nsnull, getter_AddRefs(e), nsnull);
nsnull, getter_AddRefs(e), nsnull, nsnull);
xpcc->SetException(e);
if(sz)
JS_smprintf_free(sz);

View File

@ -1384,8 +1384,11 @@ XPCConvert::ConstructException(nsresult rv, const char* message,
const char* ifaceName, const char* methodName,
nsISupports* data,
nsIException** exceptn,
const jsval *jsExceptionPtr)
JSContext* cx,
jsval* jsExceptionPtr)
{
NS_ASSERTION(!cx == !jsExceptionPtr, "Expected cx and jsExceptionPtr to cooccur.");
static const char format[] = "\'%s\' when calling method: [%s::%s]";
const char * msg = message;
char* sz = nsnull;
@ -1407,11 +1410,11 @@ XPCConvert::ConstructException(nsresult rv, const char* message,
nsresult res = nsXPCException::NewException(msg, rv, nsnull, data, exceptn);
if(NS_SUCCEEDED(res) && jsExceptionPtr && *exceptn)
if(NS_SUCCEEDED(res) && cx && jsExceptionPtr && *exceptn)
{
nsCOMPtr<nsXPCException> xpcEx = do_QueryInterface(*exceptn);
if(xpcEx)
xpcEx->SetThrownJSVal(*jsExceptionPtr);
xpcEx->StowThrownJSVal(cx, *jsExceptionPtr);
}
if(sz)
@ -1482,7 +1485,7 @@ XPCConvert::JSValToXPCException(XPCCallContext& ccx,
// it is a wrapped native, but not an exception!
return ConstructException(NS_ERROR_XPC_JS_THREW_NATIVE_OBJECT,
nsnull, ifaceName, methodName, supports,
exceptn, nsnull);
exceptn, nsnull, nsnull);
}
}
else
@ -1540,7 +1543,7 @@ XPCConvert::JSValToXPCException(XPCCallContext& ccx,
return ConstructException(NS_ERROR_XPC_JS_THREW_JS_OBJECT,
JS_GetStringBytes(str),
ifaceName, methodName, nsnull,
exceptn, &s);
exceptn, cx, &s);
}
}
@ -1548,7 +1551,7 @@ XPCConvert::JSValToXPCException(XPCCallContext& ccx,
{
return ConstructException(NS_ERROR_XPC_JS_THREW_NULL,
nsnull, ifaceName, methodName, nsnull,
exceptn, &s);
exceptn, cx, &s);
}
if(JSVAL_IS_NUMBER(s))
@ -1581,7 +1584,7 @@ XPCConvert::JSValToXPCException(XPCCallContext& ccx,
if(isResult)
return ConstructException(rv, nsnull, ifaceName, methodName,
nsnull, exceptn, &s);
nsnull, exceptn, cx, &s);
else
{
// XXX all this nsISupportsDouble code seems a little redundant
@ -1597,7 +1600,7 @@ XPCConvert::JSValToXPCException(XPCCallContext& ccx,
return NS_ERROR_FAILURE;
data->SetData(number);
rv = ConstructException(NS_ERROR_XPC_JS_THREW_NUMBER, nsnull,
ifaceName, methodName, data, exceptn, &s);
ifaceName, methodName, data, exceptn, cx, &s);
NS_RELEASE(data);
return rv;
}
@ -1611,7 +1614,7 @@ XPCConvert::JSValToXPCException(XPCCallContext& ccx,
return ConstructException(NS_ERROR_XPC_JS_THREW_STRING,
JS_GetStringBytes(str),
ifaceName, methodName, nsnull,
exceptn, &s);
exceptn, cx, &s);
return NS_ERROR_FAILURE;
}
@ -1665,7 +1668,7 @@ XPCConvert::JSErrorToXPCException(XPCCallContext& ccx,
rv = ConstructException(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS,
formattedMsg.get(), ifaceName, methodName, data,
exceptn, nsnull);
exceptn, nsnull, nsnull);
NS_RELEASE(data);
}
@ -1673,7 +1676,7 @@ XPCConvert::JSErrorToXPCException(XPCCallContext& ccx,
{
rv = ConstructException(NS_ERROR_XPC_JAVASCRIPT_ERROR,
nsnull, ifaceName, methodName, nsnull,
exceptn, nsnull);
exceptn, nsnull, nsnull);
}
return rv;
}

View File

@ -152,23 +152,21 @@ nsXPCException::~nsXPCException()
}
PRBool
nsXPCException::GetThrownJSVal(jsval *vp) const
nsXPCException::StealThrownJSVal(jsval *vp)
{
if(mThrownJSVal)
if(mThrownJSVal.IsHeld())
{
if(vp)
*vp = mThrownJSVal->GetJSVal();
*vp = mThrownJSVal.Release();
return PR_TRUE;
}
return PR_FALSE;
}
void
nsXPCException::SetThrownJSVal(jsval v)
nsXPCException::StowThrownJSVal(JSContext *cx, jsval v)
{
mThrownJSVal = JSVAL_IS_TRACEABLE(v)
? new XPCTraceableVariant(nsXPConnect::GetRuntimeInstance(), v)
: new XPCVariant(v);
if (mThrownJSVal.Hold(cx))
mThrownJSVal = v;
}
void

View File

@ -95,6 +95,7 @@
#include "nsString.h"
#include "nsReadableUtils.h"
#include "nsXPIDLString.h"
#include "nsAutoJSValHolder.h"
#include "nsThreadUtils.h"
#include "nsIJSContextStack.h"
@ -2834,7 +2835,8 @@ public:
const char* methodName,
nsISupports* data,
nsIException** exception,
const jsval *jsExceptionPtr);
JSContext* cx,
jsval *jsExceptionPtr);
static void RemoveXPCOMUCStringFinalizer();
@ -2920,8 +2922,6 @@ private:
/***************************************************************************/
class XPCVariant;
class nsXPCException :
public nsIXPCException
{
@ -2954,8 +2954,8 @@ public:
static void InitStatics() { sEverMadeOneFromFactory = JS_FALSE; }
PRBool GetThrownJSVal(jsval *vp) const;
void SetThrownJSVal(jsval v);
PRBool StealThrownJSVal(jsval* vp);
void StowThrownJSVal(JSContext* cx, jsval v);
protected:
void Reset();
@ -2970,7 +2970,7 @@ private:
nsIException* mInner;
PRBool mInitialized;
nsCOMPtr<XPCVariant> mThrownJSVal;
nsAutoJSValHolder mThrownJSVal;
static JSBool sEverMadeOneFromFactory;
};

View File

@ -284,7 +284,7 @@ XPCThrower::ThrowExceptionObject(JSContext* cx, nsIException* e)
// (see XPCConvert::ConstructException) and we are in a web
// context (i.e., not chrome), rethrow the original value.
if((xpcEx = do_QueryInterface(e)) &&
xpcEx->GetThrownJSVal(&thrown) &&
xpcEx->StealThrownJSVal(&thrown) &&
!IsCallerChrome())
{
JS_SetPendingException(cx, thrown);

View File

@ -1566,7 +1566,7 @@ pre_call_clean_up:
nsCOMPtr<nsIException> e;
XPCConvert::ConstructException(code, sz, GetInterfaceName(), name,
nsnull, getter_AddRefs(e), nsnull);
nsnull, getter_AddRefs(e), nsnull, nsnull);
xpcc->SetException(e);
if(sz)
JS_smprintf_free(sz);