Bug 1369111: Modify mscom interceptor to maintain a set of weak references to all interceptors that are currently live; r=jimm
authorAaron Klotz <aklotz@mozilla.com>
Thu, 18 May 2017 13:04:26 -0600
changeset 410732 780118d9c79c2a8e8f12724ac7d04b0170f330a7
parent 410731 09894b9ce473f681e86dfaf59ef4286c131ae669
child 410733 6e543c8a893137a588b4c6696d4c8b0920e260ab
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1369111
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1369111: Modify mscom interceptor to maintain a set of weak references to all interceptors that are currently live; r=jimm MozReview-Commit-ID: 2pyAiw53rj7 Currently we wrap every outpointer interface with its own interceptor. Even two identical interface outparams will get their own unique interceptors. Not only does this violate COM object identity, but it is also inefficient; if an interceptor already exists for a given interface, we should reuse it. This patch adds a live set: when we create a new interceptor, we first check the live set and reuse an existing interceptor if it is present. Otherwise we create a new one and then insert it into the set. The set uses thread-safe weak references to guard against races during interceptor teardown.
ipc/mscom/Interceptor.cpp
--- a/ipc/mscom/Interceptor.cpp
+++ b/ipc/mscom/Interceptor.cpp
@@ -13,30 +13,91 @@
 #include "mozilla/mscom/MainThreadInvoker.h"
 #include "mozilla/mscom/Registration.h"
 #include "mozilla/mscom/Utils.h"
 #include "MainThreadUtils.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/DebugOnly.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsDirectoryServiceUtils.h"
+#include "nsRefPtrHashtable.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace mscom {
 
+class LiveSet final
+{
+public:
+  LiveSet()
+    : mMutex("mozilla::mscom::LiveSet::mMutex")
+  {
+  }
+
+  void Lock()
+  {
+    mMutex.Lock();
+  }
+
+  void Unlock()
+  {
+    mMutex.Unlock();
+  }
+
+  void Put(IUnknown* aKey, already_AddRefed<IWeakReference> aValue)
+  {
+    mMutex.AssertCurrentThreadOwns();
+    mLiveSet.Put(aKey, Move(aValue));
+  }
+
+  RefPtr<IWeakReference> Get(IUnknown* aKey)
+  {
+    mMutex.AssertCurrentThreadOwns();
+    RefPtr<IWeakReference> result;
+    mLiveSet.Get(aKey, getter_AddRefs(result));
+    return result;
+  }
+
+  void Remove(IUnknown* aKey)
+  {
+    mMutex.AssertCurrentThreadOwns();
+    mLiveSet.Remove(aKey);
+  }
+
+  typedef BaseAutoLock<LiveSet> AutoLock;
+
+private:
+  Mutex mMutex;
+  nsRefPtrHashtable<nsPtrHashKey<IUnknown>, IWeakReference> mLiveSet;
+};
+
+static LiveSet&
+GetLiveSet()
+{
+  static LiveSet sLiveSet;
+  return sLiveSet;
+}
+
 /* static */ HRESULT
 Interceptor::Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
                     REFIID aInitialIid, void** aOutInterface)
 {
   MOZ_ASSERT(aOutInterface && aTarget && aSink);
   if (!aOutInterface) {
     return E_INVALIDARG;
   }
 
+  LiveSet::AutoLock lock(GetLiveSet());
+
+  RefPtr<IWeakReference> existingInterceptor(Move(GetLiveSet().Get(aTarget.get())));
+  if (existingInterceptor &&
+      SUCCEEDED(existingInterceptor->Resolve(aInitialIid, aOutInterface))) {
+    return S_OK;
+  }
+
   *aOutInterface = nullptr;
 
   if (!aTarget || !aSink) {
     return E_INVALIDARG;
   }
 
   RefPtr<Interceptor> intcpt(new Interceptor(aSink));
   return intcpt->GetInitialInterceptorForIID(aInitialIid, Move(aTarget),
@@ -53,16 +114,21 @@ Interceptor::Interceptor(IInterceptorSin
   RefPtr<IWeakReference> weakRef;
   if (SUCCEEDED(GetWeakReference(getter_AddRefs(weakRef)))) {
     aSink->SetInterceptor(weakRef);
   }
 }
 
 Interceptor::~Interceptor()
 {
+  { // Scope for lock
+    LiveSet::AutoLock lock(GetLiveSet());
+    GetLiveSet().Remove(mTarget.get());
+  }
+
   // This needs to run on the main thread because it releases target interface
   // reference counts which may not be thread-safe.
   MOZ_ASSERT(NS_IsMainThread());
   for (uint32_t index = 0, len = mInterceptorMap.Length(); index < len; ++index) {
     MapEntry& entry = mInterceptorMap[index];
     entry.mInterceptor = nullptr;
     entry.mTargetInterface->Release();
   }
@@ -231,20 +297,27 @@ Interceptor::GetInitialInterceptorForIID
     return hr;
   }
 
   hr = interceptor->RegisterSink(mEventSink);
   if (FAILED(hr)) {
     return hr;
   }
 
+  RefPtr<IWeakReference> 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());
 
   // 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);