Bug 1428759: Remove mutual exclusion from WeakReferenceSupport interface queries; r=Jamie

--HG--
extra : rebase_source : 1626509510d4f6018381b3f183ca40f2085f46d5

ipc/mscom/Interceptor.cpp

@@ -281,7 +281,8 @@ Interceptor::Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
 Interceptor::Interceptor(IInterceptorSink* aSink)
   : WeakReferenceSupport(WeakReferenceSupport::Flags::eDestroyOnMainThread)
   , mEventSink(aSink)
-  , mMutex("mozilla::mscom::Interceptor::mMutex")
+  , mInterceptorMapMutex("mozilla::mscom::Interceptor::mInterceptorMapMutex")
+  , mStdMarshalMutex("mozilla::mscom::Interceptor::mStdMarshalMutex")
   , mStdMarshal(nullptr)
 {
   MOZ_ASSERT(aSink);
@@ -447,7 +448,7 @@ Interceptor::DisconnectObject(DWORD dwReserved)
 Interceptor::MapEntry*
 Interceptor::Lookup(REFIID aIid)
 {
-  mMutex.AssertCurrentThreadOwns();
+  mInterceptorMapMutex.AssertCurrentThreadOwns();
 
   for (uint32_t index = 0, len = mInterceptorMap.Length(); index < len; ++index) {
     if (mInterceptorMap[index].mIID == aIid) {
@@ -461,7 +462,7 @@ HRESULT
 Interceptor::GetTargetForIID(REFIID aIid,
                              InterceptorTargetPtr<IUnknown>& aTarget)
 {
-  MutexAutoLock lock(mMutex);
+  MutexAutoLock lock(mInterceptorMapMutex);
   MapEntry* entry = Lookup(aIid);
   if (entry) {
     aTarget.reset(entry->mTargetInterface);
@@ -550,9 +551,9 @@ Interceptor::GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLiveSetLock,
   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);
+    // We must lock mInterceptorMapMutex so that nothing can race with us once
+    // we have been published to the live set.
+    MutexAutoLock lock(mInterceptorMapMutex);
 
     HRESULT hr = PublishTarget(aLiveSetLock, nullptr, aTargetIid, Move(aTarget));
     ENSURE_HR_SUCCEEDED(hr);
@@ -579,9 +580,9 @@ Interceptor::GetInitialInterceptorForIID(detail::LiveSetAutoLock& aLiveSetLock,
   hr = interceptor->RegisterSink(mEventSink);
   ENSURE_HR_SUCCEEDED(hr);
 
-  // We must lock ourselves so that nothing can race with us once we have been
-  // published to the live set.
-  AutoLock lock(*this);
+  // We must lock mInterceptorMapMutex so that nothing can race with us once we have
+  // been published to the live set.
+  MutexAutoLock lock(mInterceptorMapMutex);
 
   hr = PublishTarget(aLiveSetLock, unkInterceptor, aTargetIid, Move(aTarget));
   ENSURE_HR_SUCCEEDED(hr);
@@ -630,7 +631,7 @@ Interceptor::GetInterceptorForIID(REFIID aIid, void** aOutInterceptor)
   // interceptorIid.
 
   { // Scope for lock
-    MutexAutoLock lock(mMutex);
+    MutexAutoLock lock(mInterceptorMapMutex);
     MapEntry* entry = Lookup(interceptorIid);
     if (entry) {
       unkInterceptor = entry->mInterceptor;
@@ -697,7 +698,7 @@ Interceptor::GetInterceptorForIID(REFIID aIid, void** aOutInterceptor)
   // (5) Now that we have this new COM interceptor, insert it into the map.
 
   { // Scope for lock
-    MutexAutoLock lock(mMutex);
+    MutexAutoLock lock(mInterceptorMapMutex);
     // We might have raced with another thread, so first check that we don't
     // already have an entry for this
     MapEntry* entry = Lookup(interceptorIid);
@@ -754,7 +755,7 @@ Interceptor::QueryInterface(REFIID riid, void** ppv)
 }
 
 HRESULT
-Interceptor::ThreadSafeQueryInterface(REFIID aIid, IUnknown** aOutInterface)
+Interceptor::WeakRefQueryInterface(REFIID aIid, IUnknown** aOutInterface)
 {
   if (aIid == IID_IStdMarshalInfo) {
     detail::ReentrySentinel sentinel(this);
@@ -777,6 +778,8 @@ Interceptor::ThreadSafeQueryInterface(REFIID aIid, IUnknown** aOutInterface)
   }
 
   if (aIid == IID_IMarshal) {
+    MutexAutoLock lock(mStdMarshalMutex);
+
     HRESULT hr;
 
     if (!mStdMarshalUnk) {

ipc/mscom/Interceptor.h

@@ -127,8 +127,8 @@ private:
   MapEntry* Lookup(REFIID aIid);
   HRESULT QueryInterfaceTarget(REFIID aIid, void** aOutput,
                                TimeDuration* aOutDuration = nullptr);
-  HRESULT ThreadSafeQueryInterface(REFIID aIid,
-                                   IUnknown** aOutInterface) override;
+  HRESULT WeakRefQueryInterface(REFIID aIid,
+                                IUnknown** aOutInterface) override;
   HRESULT CreateInterceptor(REFIID aIid, IUnknown* aOuter, IUnknown** aOutput);
   REFIID MarshalAs(REFIID aIid) const;
   HRESULT PublishTarget(detail::LiveSetAutoLock& aLiveSetLock,
@@ -139,9 +139,10 @@ private:
 private:
   InterceptorTargetPtr<IUnknown>  mTarget;
   RefPtr<IInterceptorSink>  mEventSink;
-  mozilla::Mutex            mMutex; // Guards mInterceptorMap
+  mozilla::Mutex            mInterceptorMapMutex; // Guards mInterceptorMap
   // Using a nsTArray since the # of interfaces is not going to be very high
   nsTArray<MapEntry>        mInterceptorMap;
+  mozilla::Mutex            mStdMarshalMutex; // Guards mStdMarshalUnk and mStdMarshal
   RefPtr<IUnknown>          mStdMarshalUnk;
   IMarshal*                 mStdMarshal; // WEAK
 };

ipc/mscom/WeakRef.cpp

@@ -101,24 +101,6 @@ WeakReferenceSupport::WeakReferenceSupport(Flags aFlags)
   , mFlags(aFlags)
 {
   mSharedRef = new detail::SharedRef(this);
-  ::InitializeCS(mCSForQI);
-}
-
-WeakReferenceSupport::~WeakReferenceSupport()
-{
-  ::DeleteCriticalSection(&mCSForQI);
-}
-
-void
-WeakReferenceSupport::Lock()
-{
-  ::EnterCriticalSection(&mCSForQI);
-}
-
-void
-WeakReferenceSupport::Unlock()
-{
-  ::LeaveCriticalSection(&mCSForQI);
 }
 
 HRESULT
@@ -136,8 +118,7 @@ WeakReferenceSupport::QueryInterface(REFIID riid, void** ppv)
   if (riid == IID_IUnknown || riid == IID_IWeakReferenceSource) {
     punk = static_cast<IUnknown*>(this);
   } else {
-    AutoCriticalSection lock(&mCSForQI);
-    HRESULT hr = ThreadSafeQueryInterface(riid, getter_AddRefs(punk));
+    HRESULT hr = WeakRefQueryInterface(riid, getter_AddRefs(punk));
     if (FAILED(hr)) {
       return hr;
     }

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"
 
@@ -97,22 +96,15 @@ public:
 
 protected:
   explicit WeakReferenceSupport(Flags aFlags);
-  virtual ~WeakReferenceSupport();
+  virtual ~WeakReferenceSupport() = default;
 
-  virtual HRESULT ThreadSafeQueryInterface(REFIID aIid,
-                                           IUnknown** aOutInterface) = 0;
-
-  void Lock();
-  void Unlock();
-
-  typedef BaseAutoLock<WeakReferenceSupport> AutoLock;
-  friend class BaseAutoLock<WeakReferenceSupport>;
+  virtual HRESULT WeakRefQueryInterface(REFIID aIid,
+                                        IUnknown** aOutInterface) = 0;
 
 private:
   RefPtr<detail::SharedRef> mSharedRef;
   ULONG                     mRefCnt;
   Flags                     mFlags;
-  CRITICAL_SECTION          mCSForQI;
 };
 
 class WeakRef final : public IWeakReference