From 6496d65bc9689903dd28232df7e5da8952161e9a Mon Sep 17 00:00:00 2001 From: "rpotts%netscape.com" Date: Mon, 16 Jul 2001 21:26:04 +0000 Subject: [PATCH] bug #88678 (r=jst, r=dougt) The manipulation of nsProxyEventObjects is not threadsafe. git-svn-id: svn://10.0.0.236/trunk@99342 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/xpcom/proxy/src/nsProxyEvent.cpp | 2 +- mozilla/xpcom/proxy/src/nsProxyEventClass.cpp | 58 +-- .../xpcom/proxy/src/nsProxyEventObject.cpp | 419 ++++++++++-------- mozilla/xpcom/proxy/src/nsProxyEventPrivate.h | 26 +- .../xpcom/proxy/src/nsProxyObjectManager.cpp | 7 + mozilla/xpcom/threads/nsAutoLock.cpp | 4 + mozilla/xpcom/threads/nsAutoLock.h | 31 +- 7 files changed, 321 insertions(+), 226 deletions(-) diff --git a/mozilla/xpcom/proxy/src/nsProxyEvent.cpp b/mozilla/xpcom/proxy/src/nsProxyEvent.cpp index 1524e5658ec..c9e15b794fd 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEvent.cpp +++ b/mozilla/xpcom/proxy/src/nsProxyEvent.cpp @@ -298,7 +298,7 @@ nsProxyObject::Release(void) mDestQueue->PostEvent(event); return 0; } - return mRefCnt; + return count; } diff --git a/mozilla/xpcom/proxy/src/nsProxyEventClass.cpp b/mozilla/xpcom/proxy/src/nsProxyEventClass.cpp index 8e5d3b19e17..ff11ec65b0b 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEventClass.cpp +++ b/mozilla/xpcom/proxy/src/nsProxyEventClass.cpp @@ -223,11 +223,11 @@ nsProxyEventClass::CallQueryInterfaceOnProxy(nsProxyEventObject* self, REFNSIID { NS_PRECONDITION(aInstancePtr, "Requires non-null result"); - nsresult rv; + nsresult rv; + + *aInstancePtr = nsnull; // in case of error. - *aInstancePtr = nsnull; // in case of error. - // The functions we will call: QueryInterface(REFNSIID aIID, void** aInstancePtr) nsXPTCMiniVariant var[2]; @@ -238,40 +238,40 @@ nsProxyEventClass::CallQueryInterfaceOnProxy(nsProxyEventObject* self, REFNSIID nsCOMPtr interfaceInfo; const nsXPTMethodInfo *mi; - nsCOMPtr iim = getter_AddRefs(XPTI_GetInterfaceInfoManager()); + nsCOMPtr iim = getter_AddRefs(XPTI_GetInterfaceInfoManager()); - if (!iim) return NS_NOINTERFACE; - iim->GetInfoForName("nsISupports", getter_AddRefs(interfaceInfo)); + if (!iim) return NS_NOINTERFACE; + iim->GetInfoForName("nsISupports", getter_AddRefs(interfaceInfo)); interfaceInfo->GetMethodInfo(0, &mi); // 0 is QueryInterface rv = self->CallMethod(0, mi, var); - - if (NS_SUCCEEDED(rv)) - { + + if (NS_SUCCEEDED(rv)) + { nsISupports *aIdentificationObject; rv = (*aInstancePtr)->QueryInterface(kProxyObject_Identity_Class_IID, (void**)&aIdentificationObject); - if (NS_FAILED(rv)) - { - // okay, aInstancePtr was not a proxy. Lets create one. - nsProxyObjectManager *manager = nsProxyObjectManager::GetInstance(); - if (manager == nsnull) - { - NS_IF_RELEASE((*aInstancePtr)); - return NS_ERROR_FAILURE; - } - - rv = manager->GetProxyForObject(self->GetQueue(), + if (NS_FAILED(rv)) + { + // okay, aInstancePtr was not a proxy. Lets create one. + nsProxyObjectManager *manager = nsProxyObjectManager::GetInstance(); + if (manager == nsnull) + { + NS_IF_RELEASE((*aInstancePtr)); + return NS_ERROR_FAILURE; + } + + rv = manager->GetProxyForObject(self->GetQueue(), aIID, self->GetRealObject(), self->GetProxyType(), (void**) &aIdentificationObject); - } - - NS_IF_RELEASE((*aInstancePtr)); - (*aInstancePtr) = NS_STATIC_CAST(nsProxyEventObject*, aIdentificationObject); - } + } + + NS_IF_RELEASE((*aInstancePtr)); + (*aInstancePtr) = NS_STATIC_CAST(nsProxyEventObject*, aIdentificationObject); + } return rv; } @@ -313,11 +313,14 @@ nsProxyEventClass::DelegatedQueryInterface(nsProxyEventObject* self, } nsProxyEventObject* sibling; - + { + nsProxyObjectManager* manager = nsProxyObjectManager::GetInstance(); + nsAutoMonitor mon(manager->GetMonitor()); + // This includes checking for nsISupports and the iid of self. // And it also checks for other wrappers that have been constructed // for this object. - if(nsnull != (sibling = self->Find(aIID))) + if(nsnull != (sibling = self->LockedFind(aIID))) { NS_ADDREF(sibling); *aInstancePtr = (void*) sibling; @@ -345,6 +348,7 @@ nsProxyEventClass::DelegatedQueryInterface(nsProxyEventObject* self, } } } + } return CallQueryInterfaceOnProxy(self, aIID, (nsProxyEventObject**)aInstancePtr); } diff --git a/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp b/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp index 5a5de03e9f2..35782d3a30e 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp +++ b/mozilla/xpcom/proxy/src/nsProxyEventObject.cpp @@ -37,6 +37,7 @@ static NS_DEFINE_IID(kProxyObject_Identity_Class_IID, NS_PROXYEVENT_IDENTITY_CLASS_IID); + //////////////////////////////////////////////////////////////////////////////// class nsProxyEventKey : public nsHashKey @@ -152,17 +153,24 @@ nsProxyEventObject::GetNewOrUsedProxy(nsIEventQueue *destQueue, nsISupports *aObj, REFNSIID aIID) { + nsresult rv; + if (!aObj) return nsnull; nsISupports* rawObject = aObj; - // make sure that the object pass in is not a proxy. - nsCOMPtr aIdentificationObject; - nsresult rv = rawObject->QueryInterface(kProxyObject_Identity_Class_IID, getter_AddRefs(aIdentificationObject)); + // + // make sure that the object pass in is not a proxy... + // if the object *is* a proxy, then be nice and build the proxy + // for the real object... + // + nsCOMPtr identificationObject; - if (NS_SUCCEEDED(rv)) - { + rv = rawObject->QueryInterface(kProxyObject_Identity_Class_IID, + getter_AddRefs(identificationObject)); + if (NS_SUCCEEDED(rv)) { + // // ATTENTION!!!! // // If you are hitting any of the assertions in this block of code, @@ -174,160 +182,195 @@ nsProxyEventObject::GetNewOrUsedProxy(nsIEventQueue *destQueue, // a proxy from a proxy. -- dougt@netscape.com NS_ASSERTION(0, "Someone is building a proxy from a proxy"); - NS_ASSERTION(aIdentificationObject, "where did my identification object go!"); - - if (!aIdentificationObject) + NS_ASSERTION(identificationObject, "where did my identification object go!"); + if (!identificationObject) { return nsnull; + } // someone is asking us to create a proxy for a proxy. Lets get // the real object and build aproxy for that! - rawObject = aIdentificationObject->GetRealObject(); + rawObject = identificationObject->GetRealObject(); NS_ASSERTION(rawObject, "where did my real object go!"); - - if (!rawObject) + if (!rawObject) { return nsnull; + } } - // Get a class for this IID. - nsCOMPtr clazz = getter_AddRefs( nsProxyEventClass::GetNewOrUsedClass(aIID) ); - if(!clazz) - return nsnull; - - nsCOMPtr proxy; - nsCOMPtr root; - nsProxyEventObject* peo; - - // always find the native root if the |real| object. - // this must not be a nsCOMPtr since we need to make sure that we do a QI. + // + // Get the root nsISupports of the |real| object. + // nsCOMPtr rootObject; - if(NS_FAILED(rawObject->QueryInterface(NS_GET_IID(nsISupports), getter_AddRefs(rootObject)))) - return nsnull; - NS_ASSERTION(rootObject, "where did my root object go!"); - if (!rootObject) + rootObject = do_QueryInterface(rawObject, &rv); + if (NS_FAILED(rv) || !rootObject) { + NS_ASSERTION(NS_FAILED(rv), "where did my root object go!"); return nsnull; + } - /* get our hash table */ - nsProxyObjectManager *manager = nsProxyObjectManager::GetInstance(); - if (manager == nsnull) - return nsnull; - - nsHashtable *realToProxyMap = manager->GetRealObjectToProxyObjectMap(); - if (realToProxyMap == nsnull) - return nsnull; - - // we need to do make sure that we addref the passed in object as well as ensure - // that it is of the requested IID; - // this must not be a nsCOMPtr since we need to make sure that we do a QI. - - nsCOMPtr requestedInterface; - if(NS_FAILED(rawObject->QueryInterface(aIID, getter_AddRefs(requestedInterface)))) - return nsnull; - - NS_ASSERTION(requestedInterface, "where did my requestedInterface object go!"); - if (!rootObject) - return nsnull; - - // this will be our key in the hash table. - // this must not be a nsCOMPtr since we need to make sure that we do a QI. + // Get the root nsISupports of the event queue... This is used later, + // as part of the hashtable key... nsCOMPtr destQRoot = do_QueryInterface(destQueue, &rv); - if (NS_FAILED(rv) || !destQRoot) + if (NS_FAILED(rv) || !destQRoot) { return nsnull; + } + // + // Enter the proxy object creation lock. + // + // This lock protects thev linked list which chains proxies together + // (ie. mRoot and mNext) and ensures that there is no hashtable contention + // between the time that a proxy is looked up (and not found) in the + // hashtable and then added... + // + nsProxyObjectManager *manager = nsProxyObjectManager::GetInstance(); + if (!manager) { + return nsnull; + } + + nsAutoMonitor mon(manager->GetMonitor()); + + // Get the hash table containing root proxy objects... + nsHashtable *realToProxyMap = manager->GetRealObjectToProxyObjectMap(); + if (!realToProxyMap) { + return nsnull; + } + + // Now, lookup the root nsISupports of the raw object in the hashtable + // The key consists of 3 pieces of information: + // - root nsISupports of the raw object + // - event queue of the current thread + // - type of proxy being constructed... + // nsProxyEventKey rootkey(rootObject.get(), destQRoot.get(), proxyType); + nsCOMPtr rootProxy; + nsCOMPtr proxy; + nsProxyEventObject* peo; + // find in our hash table - root = (nsProxyEventObject*) realToProxyMap->Get(&rootkey); - if(root) - { - proxy = root->Find(aIID); - - if(proxy) - { - peo = proxy; + rootProxy = (nsProxyEventObject*) realToProxyMap->Get(&rootkey); + + if(rootProxy) { + // + // At least one proxy has already been created for this raw object... + // + // Look for the specific interface proxy in the list off of + // the root proxy... + // + peo = rootProxy->LockedFind(aIID); + + if(peo) { + // An existing proxy is available... So use it. NS_ADDREF(peo); return peo; } } - else - { + else { // build the root proxy - if (rawObject == rootObject.get()) - { - // the root will do double duty as the interface wrapper - peo = new nsProxyEventObject(destQueue, - proxyType, - requestedInterface, - clazz, - nsnull); - - proxy = do_QueryInterface(peo); - - if(proxy) - { - realToProxyMap->Put(&rootkey, peo); - peo = proxy; - NS_ADDREF(peo); - return peo; - } - } - else - { - // just a root proxy - nsCOMPtr rootClazz = getter_AddRefs ( nsProxyEventClass::GetNewOrUsedClass( - NS_GET_IID(nsISupports)) ); - - if (!rootClazz) - { - return nsnull; - } - - peo = new nsProxyEventObject(destQueue, - proxyType, - rootObject, - rootClazz, - nsnull); + nsCOMPtr rootClazz; - if(!peo) - { - return nsnull; - } - - root = do_QueryInterface(peo); - - realToProxyMap->Put(&rootkey, peo); - } - } - // at this point we have a root and may need to build the specific proxy - NS_ASSERTION(root,"bad root"); - NS_ASSERTION(clazz,"bad clazz"); - - if(!proxy) - { - peo = new nsProxyEventObject(destQueue, - proxyType, - requestedInterface, - clazz, - root); - - proxy = do_QueryInterface(peo); - - if(!proxy) - { + rootClazz = dont_AddRef(nsProxyEventClass::GetNewOrUsedClass( + NS_GET_IID(nsISupports))); + if (!rootClazz) { return nsnull; } + + peo = new nsProxyEventObject(destQueue, + proxyType, + rootObject, + rootClazz, + nsnull); + if(!peo) { + // Ouch... Out of memory! + return nsnull; + } + + // Add this root proxy into the hash table... + realToProxyMap->Put(&rootkey, peo); + + if(aIID.Equals(NS_GET_IID(nsISupports))) { + // + // Since the requested proxy is for the nsISupports interface of + // the raw object, use the new root proxy as the specific proxy + // too... + // + NS_ADDREF(peo); + return peo; + } + + // This assignment is an owning reference to the new ProxyEventObject. + // So, it will automatically get deleted if any subsequent early + // returns are taken... + rootProxy = do_QueryInterface(peo); } - proxy->mNext = root->mNext; - root->mNext = proxy; + // + // at this point we have a proxy for the root nsISupports (ie. rootProxy) + // but we need to build the specific proxy for this interface... + // + NS_ASSERTION(rootProxy, "What happened to the root proxy!"); + + // Get a class for this IID. + nsCOMPtr proxyClazz; + + proxyClazz = dont_AddRef(nsProxyEventClass::GetNewOrUsedClass(aIID)); + if(!proxyClazz) { + return nsnull; + } + + // Get the raw interface for this IID + nsCOMPtr rawInterface; + + rv = rawObject->QueryInterface(aIID, getter_AddRefs(rawInterface)); + if (NS_FAILED(rv) || !rawInterface) { + NS_ASSERTION(NS_FAILED(rv), "where did my rawInterface object go!"); + return nsnull; + } + + peo = new nsProxyEventObject(destQueue, + proxyType, + rawInterface, + proxyClazz, + rootProxy); + if (!peo) { + // Ouch... Out of memory! + return nsnull; + } + + // + // Add the new specific proxy to the head of the list of proxies hanging + // off of the rootProxy... + // + peo->mNext = rootProxy->mNext; + rootProxy->mNext = peo; - peo = proxy; NS_ADDREF(peo); return peo; } +nsProxyEventObject* nsProxyEventObject::LockedFind(REFNSIID aIID) +{ + if(aIID.Equals(mClass->GetProxiedIID())) { + return this; + } + + if(aIID.Equals(NS_GET_IID(nsISupports))) { + return this; + } + + nsProxyEventObject* cur = (mRoot ? mRoot : mNext); + while(cur) { + if(aIID.Equals(cur->GetClass()->GetProxiedIID())) { + return cur; + } + cur = cur->mNext; + } + + return nsnull; +} + nsProxyEventObject::nsProxyEventObject() : mNext(nsnull) { @@ -358,29 +401,35 @@ nsProxyEventObject::~nsProxyEventObject() #ifdef DEBUG_xpcom_proxy DebugDump("Delete", 0); #endif - if (mRoot != nsnull) - { + if (mRoot) { + // + // This proxy is not the root interface so it must be removed + // from the chain of proxies... + // nsProxyEventObject* cur = mRoot; - while(1) - { - if(cur->mNext == this) - { + while(cur) { + if(cur->mNext == this) { cur->mNext = mNext; + mNext = nsnull; break; } cur = cur->mNext; - NS_ASSERTION(cur, "failed to find wrapper in its own chain"); } + NS_ASSERTION(cur, "failed to find wrapper in its own chain"); } - else - { - if (! nsProxyObjectManager::IsManagerShutdown()) - { - nsCOMPtr manager = nsProxyObjectManager::GetInstance(); + else { + // + // This proxy is for the root interface. Each proxy in the chain + // has a strong reference to the root... So, when its refcount goes + // to zero, it safe to remove it because no proxies are in its chain. + // + if (! nsProxyObjectManager::IsManagerShutdown()) { + nsProxyObjectManager* manager = nsProxyObjectManager::GetInstance(); nsHashtable *realToProxyMap = manager->GetRealObjectToProxyObjectMap(); - if (realToProxyMap != nsnull) - { + NS_ASSERTION(!mNext, "There are still proxies in the chain!"); + + if (realToProxyMap != nsnull) { nsCOMPtr rootObject = do_QueryInterface(mProxyObject->mRealObject); nsCOMPtr rootQueue = do_QueryInterface(mProxyObject->mDestQueue); nsProxyEventKey key(rootObject, rootQueue, mProxyObject->mProxyType); @@ -389,22 +438,56 @@ nsProxyEventObject::~nsProxyEventObject() } } } + // I am worried about ordering. // do not remove assignments. - mProxyObject = 0; - mClass = 0; + mProxyObject = nsnull; + mClass = nsnull; NS_IF_RELEASE(mRoot); } +// +// nsISupports implementation... +// + NS_IMPL_THREADSAFE_ADDREF(nsProxyEventObject); -NS_IMPL_THREADSAFE_RELEASE(nsProxyEventObject); + +NS_IMETHODIMP_(nsrefcnt) +nsProxyEventObject::Release(void) +{ + // + // Be pessimistic about whether the manager or even the monitor exist... + // This is to protect against shutdown issues where a proxy object could + // be destroyed after (or while) the Proxy Manager is being destroyed... + // + nsProxyObjectManager* manager = nsProxyObjectManager::GetInstance(); + nsAutoMonitor mon(manager ? manager->GetMonitor() : nsnull); + + nsrefcnt count; + NS_PRECONDITION(0 != mRefCnt, "dup release"); + // Decrement atomically - in case the Proxy Object Manager has already + // been deleted and the monitor is unavailable... + count = PR_AtomicDecrement((PRInt32 *)&mRefCnt); + NS_LOG_RELEASE(this, count, "nsProxyEventObject"); + if (0 == count) { + mRefCnt = 1; /* stabilize */ + // + // Remove the proxy from the hashtable (if necessary) or its + // proxy chain. This must be done inside of the proxy lock to + // prevent GetNewOrUsedProxy(...) from ressurecting it... + // + NS_DELETEXPCOM(this); + return 0; + } + return count; +} NS_IMETHODIMP nsProxyEventObject::QueryInterface(REFNSIID aIID, void** aInstancePtr) { if( aIID.Equals(GetIID()) ) { - *aInstancePtr = (void*) ( (nsISupports*)this ); //todo should use standard cast. + *aInstancePtr = NS_STATIC_CAST(nsISupports*, this); NS_ADDREF_THIS(); return NS_OK; } @@ -412,43 +495,21 @@ nsProxyEventObject::QueryInterface(REFNSIID aIID, void** aInstancePtr) return mClass->DelegatedQueryInterface(this, aIID, aInstancePtr); } -nsProxyEventObject* -nsProxyEventObject::Find(REFNSIID aIID) -{ - if(aIID.Equals(GetClass()->GetProxiedIID())) - { - return this; - } - - if(aIID.Equals(NS_GET_IID(nsISupports))) - { - return this; - } - - nsProxyEventObject* cur = (mRoot ? mRoot : this); - - do - { - if(aIID.Equals(GetClass()->GetProxiedIID())) - { - return cur; - } - - } while(NULL != (cur = cur->mNext)); - - return NULL; -} - +// +// nsXPTCStubBase implementation... +// NS_IMETHODIMP nsProxyEventObject::GetInterfaceInfo(nsIInterfaceInfo** info) { NS_ENSURE_ARG_POINTER(info); - NS_ASSERTION(GetClass(), "proxy without class"); - NS_ASSERTION(GetClass()->GetInterfaceInfo(), "proxy class without interface"); - if(!(*info = GetClass()->GetInterfaceInfo())) + *info = mClass->GetInterfaceInfo(); + + NS_ASSERTION(*info, "proxy class without interface"); + if (!*info) { return NS_ERROR_UNEXPECTED; + } NS_ADDREF(*info); return NS_OK; @@ -459,8 +520,16 @@ nsProxyEventObject::CallMethod(PRUint16 methodIndex, const nsXPTMethodInfo* info, nsXPTCMiniVariant * params) { - if (mProxyObject) - return mProxyObject->Post(methodIndex, (nsXPTMethodInfo*)info, params, GetClass()->GetInterfaceInfo()); + nsresult rv; - return NS_ERROR_NULL_POINTER; + if (mProxyObject) { + rv = mProxyObject->Post(methodIndex, + (nsXPTMethodInfo*)info, + params, + mClass->GetInterfaceInfo()); + } else { + rv = NS_ERROR_NULL_POINTER; + } + + return rv; } diff --git a/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h b/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h index 04fc992168e..531b81593f3 100644 --- a/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h +++ b/mozilla/xpcom/proxy/src/nsProxyEventPrivate.h @@ -117,30 +117,28 @@ public: nsProxyEventObject(nsIEventQueue *destQueue, PRInt32 proxyType, nsISupports* aObj, - nsProxyEventClass* aClass, + nsProxyEventClass* aClass, nsProxyEventObject* root); virtual ~nsProxyEventObject(); - nsProxyEventObject* Find(REFNSIID aIID); + nsProxyEventObject* LockedFind(REFNSIID aIID); #ifdef DEBUG_xpcom_proxy void DebugDump(const char * message, PRUint32 hashKey); #endif protected: - PRLock *mLock; + void LockedRemoveProxy(); - nsCOMPtr mClass; - nsCOMPtr mProxyObject; - /* - nsProxyEventObject are - incomplete classes. do not try changing - them into nsCOMPtr's, or you will find - that it wont work :-) - */ - +protected: + nsCOMPtr mClass; + nsCOMPtr mProxyObject; + + // Owning reference... nsProxyEventObject *mRoot; + + // Weak reference... nsProxyEventObject *mNext; }; @@ -170,12 +168,14 @@ public: nsHashtable* GetRealObjectToProxyObjectMap() const { return mProxyObjectMap;} nsHashtable* GetIIDToProxyClassMap() const { return mProxyClassMap; } - + + PRMonitor* GetMonitor() const { return mProxyCreationMonitor; } private: static nsProxyObjectManager* mInstance; nsHashtable *mProxyObjectMap; nsHashtable *mProxyClassMap; + PRMonitor *mProxyCreationMonitor; }; diff --git a/mozilla/xpcom/proxy/src/nsProxyObjectManager.cpp b/mozilla/xpcom/proxy/src/nsProxyObjectManager.cpp index 184a61f0dfe..8520dd6bf64 100644 --- a/mozilla/xpcom/proxy/src/nsProxyObjectManager.cpp +++ b/mozilla/xpcom/proxy/src/nsProxyObjectManager.cpp @@ -104,6 +104,8 @@ nsProxyObjectManager::nsProxyObjectManager() mProxyClassMap = new nsHashtable(256, PR_TRUE); mProxyObjectMap = new nsHashtable(256, PR_TRUE); + + mProxyCreationMonitor = PR_NewMonitor(); } static PRBool PurgeProxyClasses(nsHashKey *aKey, void *aData, void* closure) @@ -122,6 +124,11 @@ nsProxyObjectManager::~nsProxyObjectManager() } delete mProxyObjectMap; + + if (mProxyCreationMonitor) { + PR_DestroyMonitor(mProxyCreationMonitor); + } + nsProxyObjectManager::mInstance = nsnull; } diff --git a/mozilla/xpcom/threads/nsAutoLock.cpp b/mozilla/xpcom/threads/nsAutoLock.cpp index 5b67ccb59d7..388aa0273f0 100644 --- a/mozilla/xpcom/threads/nsAutoLock.cpp +++ b/mozilla/xpcom/threads/nsAutoLock.cpp @@ -299,6 +299,7 @@ void nsAutoMonitor::Enter() (void) PR_SetThreadPrivate(LockStackTPI, this); #endif PR_EnterMonitor(mMonitor); + mLockCount += 1; } void nsAutoMonitor::Exit() @@ -308,6 +309,7 @@ void nsAutoMonitor::Exit() #endif PRStatus status = PR_ExitMonitor(mMonitor); NS_ASSERTION(status == PR_SUCCESS, "PR_ExitMonitor failed"); + mLockCount -= 1; } // XXX we don't worry about cached monitors being destroyed behind our back. @@ -324,6 +326,7 @@ void nsAutoCMonitor::Enter() (void) PR_SetThreadPrivate(LockStackTPI, this); #endif PR_CEnterMonitor(mLockObject); + mLockCount += 1; } void nsAutoCMonitor::Exit() @@ -333,4 +336,5 @@ void nsAutoCMonitor::Exit() #endif PRStatus status = PR_CExitMonitor(mLockObject); NS_ASSERTION(status == PR_SUCCESS, "PR_CExitMonitor failed"); + mLockCount -= 1; } diff --git a/mozilla/xpcom/threads/nsAutoLock.h b/mozilla/xpcom/threads/nsAutoLock.h index e6b7bd6a1ee..f1ec6183d0a 100644 --- a/mozilla/xpcom/threads/nsAutoLock.h +++ b/mozilla/xpcom/threads/nsAutoLock.h @@ -178,18 +178,24 @@ public: nsAutoMonitor(PRMonitor* mon) : nsAutoLockBase((void*)mon, eAutoMonitor), - mMonitor(mon) + mMonitor(mon), mLockCount(0) { NS_ASSERTION(mMonitor, "null monitor"); - PR_EnterMonitor(mMonitor); + if (mMonitor) { + PR_EnterMonitor(mMonitor); + mLockCount = 1; + } } ~nsAutoMonitor() { + NS_ASSERTION(mMonitor, "null monitor"); + if (mMonitor && mLockCount) { #ifdef DEBUG - PRStatus status = + PRStatus status = #endif - PR_ExitMonitor(mMonitor); - NS_ASSERTION(status == PR_SUCCESS, "PR_ExitMonitor failed"); + PR_ExitMonitor(mMonitor); + NS_ASSERTION(status == PR_SUCCESS, "PR_ExitMonitor failed"); + } } void Enter(); @@ -212,6 +218,7 @@ public: private: PRMonitor* mMonitor; + PRInt32 mLockCount; // Not meant to be implemented. This makes it a compiler error to // construct or assign an nsAutoLock object incorrectly. @@ -245,18 +252,21 @@ class NS_COM nsAutoCMonitor : public nsAutoLockBase { public: nsAutoCMonitor(void* lockObject) : nsAutoLockBase(lockObject, eAutoCMonitor), - mLockObject(lockObject) + mLockObject(lockObject), mLockCount(0) { NS_ASSERTION(lockObject, "null lock object"); PR_CEnterMonitor(mLockObject); + mLockCount = 1; } ~nsAutoCMonitor() { + if (mLockCount) { #ifdef DEBUG - PRStatus status = + PRStatus status = #endif - PR_CExitMonitor(mLockObject); - NS_ASSERTION(status == PR_SUCCESS, "PR_CExitMonitor failed"); + PR_CExitMonitor(mLockObject); + NS_ASSERTION(status == PR_SUCCESS, "PR_CExitMonitor failed"); + } } void Enter(); @@ -278,7 +288,8 @@ public: } private: - void* mLockObject; + void* mLockObject; + PRInt32 mLockCount; // Not meant to be implemented. This makes it a compiler error to // construct or assign an nsAutoLock object incorrectly.