diff --git a/ipc/mscom/Interceptor.cpp b/ipc/mscom/Interceptor.cpp index bf216fabba31..88d1d7efadaf 100644 --- a/ipc/mscom/Interceptor.cpp +++ b/ipc/mscom/Interceptor.cpp @@ -305,7 +305,6 @@ Interceptor::MapEntry* Interceptor::Lookup(REFIID aIid) { mMutex.AssertCurrentThreadOwns(); - for (uint32_t index = 0, len = mInterceptorMap.Length(); index < len; ++index) { if (mInterceptorMap[index].mIID == aIid) { return &mInterceptorMap[index]; @@ -368,57 +367,15 @@ Interceptor::CreateInterceptor(REFIID aIid, IUnknown* aOuter, IUnknown** aOutput } HRESULT -Interceptor::PublishTarget(detail::LiveSetAutoLock& aLiveSetLock, - RefPtr aInterceptor, - REFIID aTargetIid, - STAUniquePtr aTarget) -{ - RefPtr weakRef; - HRESULT hr = GetWeakReference(getter_AddRefs(weakRef)); - if (FAILED(hr)) { - return hr; - } - - // mTarget is a weak reference to aTarget. This is safe because we transfer - // ownership of aTarget into mInterceptorMap which remains live for the - // lifetime of this Interceptor. - mTarget = ToInterceptorTargetPtr(aTarget); - GetLiveSet().Put(mTarget.get(), weakRef.forget()); - - // Now we transfer aTarget's ownership into mInterceptorMap. - mInterceptorMap.AppendElement(MapEntry(aTargetIid, - aInterceptor, - aTarget.release())); - - // Release the live set lock because subsequent operations may post work to - // the main thread, creating potential for deadlocks. - aLiveSetLock.Unlock(); - return S_OK; -} - -HRESULT -Interceptor::GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLiveSetLock, +Interceptor::GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLock, REFIID aTargetIid, STAUniquePtr aTarget, void** aOutInterceptor) { MOZ_ASSERT(aOutInterceptor); - MOZ_ASSERT(aTargetIid != IID_IMarshal); + MOZ_ASSERT(aTargetIid != IID_IUnknown && aTargetIid != IID_IMarshal); MOZ_ASSERT(!IsProxy(aTarget.get())); - if (aTargetIid == IID_IUnknown) { - // We must lock ourselves so that nothing can race with us once we have been - // published to the live set. - AutoLock lock(*this); - - HRESULT hr = PublishTarget(aLiveSetLock, nullptr, aTargetIid, Move(aTarget)); - if (FAILED(hr)) { - return hr; - } - - return QueryInterface(aTargetIid, aOutInterceptor); - } - // Raise the refcount for stabilization purposes during aggregation RefPtr kungFuDeathGrip(static_cast( static_cast(this))); @@ -442,15 +399,27 @@ Interceptor::GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLiveSetLock, return hr; } - // We must lock ourselves so that nothing can race with us once we have been - // published to the live set. - AutoLock lock(*this); - - hr = PublishTarget(aLiveSetLock, unkInterceptor, aTargetIid, Move(aTarget)); + RefPtr weakRef; + hr = GetWeakReference(getter_AddRefs(weakRef)); if (FAILED(hr)) { return hr; } + // mTarget is a weak reference to aTarget. This is safe because we transfer + // ownership of aTarget into mInterceptorMap which remains live for the + // lifetime of this Interceptor. + mTarget = ToInterceptorTargetPtr(aTarget); + GetLiveSet().Put(mTarget.get(), weakRef.forget()); + + // Release the live set lock because GetInterceptorForIID will post work to + // the main thread, creating potential for deadlocks. + aLock.Unlock(); + + // Now we transfer aTarget's ownership into mInterceptorMap. + mInterceptorMap.AppendElement(MapEntry(aTargetIid, + unkInterceptor, + aTarget.release())); + if (mEventSink->MarshalAs(aTargetIid) == aTargetIid) { return unkInterceptor->QueryInterface(aTargetIid, aOutInterceptor); } diff --git a/ipc/mscom/Interceptor.h b/ipc/mscom/Interceptor.h index 7214a0e6592d..7e09528f73d1 100644 --- a/ipc/mscom/Interceptor.h +++ b/ipc/mscom/Interceptor.h @@ -120,7 +120,7 @@ private: private: explicit Interceptor(IInterceptorSink* aSink); ~Interceptor(); - HRESULT GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLiveSetLock, + HRESULT GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLock, REFIID aTargetIid, STAUniquePtr aTarget, void** aOutInterface); @@ -129,10 +129,6 @@ private: HRESULT ThreadSafeQueryInterface(REFIID aIid, IUnknown** aOutInterface) override; HRESULT CreateInterceptor(REFIID aIid, IUnknown* aOuter, IUnknown** aOutput); - HRESULT PublishTarget(detail::LiveSetAutoLock& aLiveSetLock, - RefPtr aInterceptor, - REFIID aTargetIid, - STAUniquePtr aTarget); private: InterceptorTargetPtr mTarget; diff --git a/ipc/mscom/WeakRef.cpp b/ipc/mscom/WeakRef.cpp index ffdb89be4dfa..cf71d2faf1d2 100644 --- a/ipc/mscom/WeakRef.cpp +++ b/ipc/mscom/WeakRef.cpp @@ -109,18 +109,6 @@ WeakReferenceSupport::~WeakReferenceSupport() ::DeleteCriticalSection(&mCSForQI); } -void -WeakReferenceSupport::Lock() -{ - ::EnterCriticalSection(&mCSForQI); -} - -void -WeakReferenceSupport::Unlock() -{ - ::LeaveCriticalSection(&mCSForQI); -} - HRESULT WeakReferenceSupport::QueryInterface(REFIID riid, void** ppv) { diff --git a/ipc/mscom/WeakRef.h b/ipc/mscom/WeakRef.h index fcc623e430f1..99c0886b9e82 100644 --- a/ipc/mscom/WeakRef.h +++ b/ipc/mscom/WeakRef.h @@ -12,7 +12,6 @@ #include "mozilla/Assertions.h" #include "mozilla/Atomics.h" -#include "mozilla/Mutex.h" #include "mozilla/RefPtr.h" #include "nsISupportsImpl.h" @@ -102,12 +101,6 @@ protected: virtual HRESULT ThreadSafeQueryInterface(REFIID aIid, IUnknown** aOutInterface) = 0; - void Lock(); - void Unlock(); - - typedef BaseAutoLock AutoLock; - friend class AutoLock; - private: RefPtr mSharedRef; ULONG mRefCnt;