From c8912b7c4e2f974d61993e7b8db15ffba0f58550 Mon Sep 17 00:00:00 2001 From: "darin%meer.net" Date: Tue, 16 May 2006 20:17:36 +0000 Subject: [PATCH] fixes bug 337492 "xpcom proxies may release proxied object on random threads" r=bsmedberg git-svn-id: svn://10.0.0.236/trunk@196698 18797224-902f-48f8-a5cc-f745e15eee43 --- .../proxy/public/nsIProxyObjectManager.idl | 4 +- mozilla/xpcom/proxy/src/nsProxyEvent.cpp | 8 ++- .../xpcom/proxy/src/nsProxyEventObject.cpp | 59 +++++++++++++++---- mozilla/xpcom/proxy/src/nsProxyEventPrivate.h | 2 + 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/mozilla/xpcom/proxy/public/nsIProxyObjectManager.idl b/mozilla/xpcom/proxy/public/nsIProxyObjectManager.idl index 16f2e5e3b9c..723d2e4b7f1 100644 --- a/mozilla/xpcom/proxy/public/nsIProxyObjectManager.idl +++ b/mozilla/xpcom/proxy/public/nsIProxyObjectManager.idl @@ -93,7 +93,9 @@ interface nsIProxyObjectManager : nsISupports * Identifies the interface being proxied. The given object must QI to * this type. * @param object - * The object being proxied. + * The object being proxied. The AddRef and QueryInterface methods for + * this object will be called on the current thread, but this object will + * only be released on the target thread. * @param proxyType * Specifies the type of proxy to construct. Either INVOKE_SYNC or * INVOKE_ASYNC must be specified. FORCE_PROXY_CREATION may be bit-wise diff --git a/mozilla/xpcom/proxy/src/nsProxyEvent.cpp b/mozilla/xpcom/proxy/src/nsProxyEvent.cpp index 41277034601..525f93e612f 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEvent.cpp +++ b/mozilla/xpcom/proxy/src/nsProxyEvent.cpp @@ -52,6 +52,7 @@ #include "nsProxyEvent.h" #include "nsProxyEventPrivate.h" +#include "nsProxyRelease.h" #include "nsIProxyObjectManager.h" #include "nsCRT.h" @@ -421,7 +422,12 @@ nsProxyObject::~nsProxyObject() // I am worried about order of destruction here. // do not remove assignments. - mRealObject = 0; + // Proxy the release of mRealObject to protect against it being deleted on + // the wrong thread. + nsISupports *doomed = nsnull; + mRealObject.swap(doomed); + NS_ProxyRelease(mTarget, doomed); + mTarget = 0; } diff --git a/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp b/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp index f1c577dbf4e..98420589a1f 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp +++ b/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp @@ -43,6 +43,8 @@ #include "nsProxyEvent.h" #include "nsIProxyObjectManager.h" #include "nsProxyEventPrivate.h" +#include "nsProxyRelease.h" +#include "nsThreadUtils.h" #include "nsServiceManagerUtils.h" @@ -57,6 +59,36 @@ static NS_DEFINE_IID(kProxyObject_Identity_Class_IID, NS_PROXYEVENT_IDENTITY_CLA //////////////////////////////////////////////////////////////////////////////// +// Make this more generic and move it someplace else so that it can be reused! +template +class nsProxyReleaseCOMPtr : public nsCOMPtr { +public: + typedef nsProxyReleaseCOMPtr self_type; + + nsProxyReleaseCOMPtr(nsIEventTarget *target) : mTarget(target) { + } + + ~nsProxyReleaseCOMPtr() { + if (get() && mTarget != NS_GetCurrentThread()) { + nsISupports *doomed = nsnull; + swap(doomed); + NSCAP_LOG_RELEASE(this, doomed); + NS_ProxyRelease(mTarget, doomed); + } + } + + self_type& + operator=( const nsQueryInterfaceWithError& rhs ) { + *NS_STATIC_CAST(nsCOMPtr*, this) = rhs; + return *this; + } + +private: + nsIEventTarget *mTarget; // weak ptr +}; + +//////////////////////////////////////////////////////////////////////////////// + class nsProxyEventKey : public nsHashKey { public: @@ -124,9 +156,8 @@ nsProxyEventObject::DebugDump(const char * message, PRUint32 hashKey) PRBool isRoot = mRoot == nsnull; printf("%s wrapper around @ %x\n", isRoot ? "ROOT":"non-root\n", GetRealObject()); - nsCOMPtr rootObject = do_QueryInterface(mProxyObject->mRealObject); nsCOMPtr rootQueue = do_QueryInterface(mProxyObject->mTarget); - nsProxyEventKey key(rootObject, rootQueue, mProxyObject->mProxyType); + nsProxyEventKey key(mRootObj, rootQueue, mProxyObject->mProxyType); printf("Hashkey: %d\n", key.HashCode()); char* name; @@ -217,7 +248,7 @@ nsProxyEventObject::GetNewOrUsedProxy(nsIEventTarget *target, // // Get the root nsISupports of the |real| object. // - nsCOMPtr rootObject; + nsProxyReleaseCOMPtr rootObject(target); rootObject = do_QueryInterface(rawObject, &rv); if (NS_FAILED(rv) || !rootObject) { @@ -296,6 +327,7 @@ nsProxyEventObject::GetNewOrUsedProxy(nsIEventTarget *target, peo = new nsProxyEventObject(target, proxyType, rootObject, + rootObject, rootClazz, nsnull); if(!peo) { @@ -337,18 +369,19 @@ nsProxyEventObject::GetNewOrUsedProxy(nsIEventTarget *target, } // Get the raw interface for this IID - nsCOMPtr rawInterface; + nsProxyReleaseCOMPtr rawInterface(target); - rv = rawObject->QueryInterface(aIID, getter_AddRefs(rawInterface)); + rawInterface = do_QueryInterface(rawObject, &rv); if (NS_FAILED(rv) || !rawInterface) { NS_ASSERTION(NS_FAILED(rv), "where did my rawInterface object go!"); return nsnull; } - peo = new nsProxyEventObject(target, - proxyType, - rawInterface, - proxyClazz, + peo = new nsProxyEventObject(target, + proxyType, + rawInterface, + rootObject, + proxyClazz, rootProxy); if (!peo) { // Ouch... Out of memory! @@ -397,14 +430,17 @@ nsProxyEventObject::nsProxyEventObject() nsProxyEventObject::nsProxyEventObject(nsIEventTarget *target, PRInt32 proxyType, nsISupports* aObj, + nsISupports* aRootObj, nsProxyEventClass* aClass, nsProxyEventObject* root) : mClass(aClass), + mRootObj(aRootObj), mRoot(root), mNext(nsnull) { NS_IF_ADDREF(mRoot); + // XXX protect against OOM errors mProxyObject = new nsProxyObject(target, proxyType, aObj); #ifdef DEBUG_xpcom_proxy @@ -446,9 +482,8 @@ nsProxyEventObject::~nsProxyEventObject() NS_ASSERTION(!mNext, "There are still proxies in the chain!"); if (realToProxyMap != nsnull) { - nsCOMPtr rootObject = do_QueryInterface(mProxyObject->mRealObject); - nsCOMPtr rootQueue = do_QueryInterface(mProxyObject->mTarget); - nsProxyEventKey key(rootObject, rootQueue, mProxyObject->mProxyType); + nsCOMPtr rootTarget = do_QueryInterface(mProxyObject->mTarget); + nsProxyEventKey key(mRootObj, rootTarget, mProxyObject->mProxyType); #ifdef DEBUG_dougt void* value = #endif diff --git a/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h b/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h index 42f1a134b2a..0fc5c5aca34 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h +++ b/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h @@ -133,6 +133,7 @@ public: nsProxyEventObject(nsIEventTarget *target, PRInt32 proxyType, nsISupports* aObj, + nsISupports* aRootObj, // result of QI(nsISupports) nsProxyEventClass* aClass, nsProxyEventObject* root); @@ -151,6 +152,7 @@ protected: protected: nsCOMPtr mClass; nsRefPtr mProxyObject; + nsISupports *mRootObj; // weak pointer // Owning reference... nsProxyEventObject *mRoot;