Bug 1442523: mscom::Interceptor: Don't dispatch QI calls to the main thread while creating a marshaler. r=aklotz
authorJames Teh <jteh@mozilla.com>
Tue, 06 Mar 2018 17:08:15 +1000
changeset 406764 b62ac052aa5a15669ce7f10934377afb4c9082dd
parent 406763 3cee5c02db0a065f081989d3e0514563aac355ab
child 406765 a7029424d92a7f65fade0c014136fe5bca3ce8f8
push id60688
push useraklotz@mozilla.com
push dateTue, 06 Mar 2018 18:07:46 +0000
treeherderautoland@b62ac052aa5a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz
bugs1442523
milestone60.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 1442523: mscom::Interceptor: Don't dispatch QI calls to the main thread while creating a marshaler. r=aklotz COM queries for special interfaces such as IFastRundown when creating a marshaler. We don't want these being dispatched to the main thread, since this would cause a deadlock on mStdMarshalMutex if the main thread is also querying for IMarshal. MozReview-Commit-ID: EQcN8Zhewjh
ipc/mscom/Interceptor.cpp
ipc/mscom/Interceptor.h
--- a/ipc/mscom/Interceptor.cpp
+++ b/ipc/mscom/Interceptor.cpp
@@ -226,16 +226,18 @@ private:
 
 static detail::LiveSet&
 GetLiveSet()
 {
   static detail::LiveSet sLiveSet;
   return sLiveSet;
 }
 
+MOZ_THREAD_LOCAL(bool) Interceptor::tlsCreatingStdMarshal;
+
 /* static */ HRESULT
 Interceptor::Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
                     REFIID aInitialIid, void** aOutInterface)
 {
   MOZ_ASSERT(aOutInterface && aTarget && aSink);
   if (!aOutInterface) {
     return E_INVALIDARG;
   }
@@ -266,16 +268,19 @@ Interceptor::Create(STAUniquePtr<IUnknow
 
 Interceptor::Interceptor(IInterceptorSink* aSink)
   : WeakReferenceSupport(WeakReferenceSupport::Flags::eDestroyOnMainThread)
   , mEventSink(aSink)
   , mInterceptorMapMutex("mozilla::mscom::Interceptor::mInterceptorMapMutex")
   , mStdMarshalMutex("mozilla::mscom::Interceptor::mStdMarshalMutex")
   , mStdMarshal(nullptr)
 {
+  static const bool kHasTls = tlsCreatingStdMarshal.init();
+  MOZ_ASSERT(kHasTls);
+
   MOZ_ASSERT(aSink);
   RefPtr<IWeakReference> weakRef;
   if (SUCCEEDED(GetWeakReference(getter_AddRefs(weakRef)))) {
     aSink->SetInterceptor(weakRef);
   }
 }
 
 Interceptor::~Interceptor()
@@ -714,16 +719,27 @@ Interceptor::GetInterceptorForIID(REFIID
 
 HRESULT
 Interceptor::QueryInterfaceTarget(REFIID aIid, void** aOutput,
                                   TimeDuration* aOutDuration)
 {
   // NB: This QI needs to run on the main thread because the target object
   // is probably Gecko code that is not thread-safe. Note that this main
   // thread invocation is *synchronous*.
+  if (!NS_IsMainThread() && tlsCreatingStdMarshal.get()) {
+    mStdMarshalMutex.AssertCurrentThreadOwns();
+    // COM queries for special interfaces such as IFastRundown when creating a
+    // marshaler. We don't want these being dispatched to the main thread,
+    // since this would cause a deadlock on mStdMarshalMutex if the main
+    // thread is also querying for IMarshal. If we do need to respond to these
+    // special interfaces, this should be done before this point; e.g. in
+    // Interceptor::QueryInterface like we do for INoMarshal.
+    return E_NOINTERFACE;
+  }
+
   MainThreadInvoker invoker;
   HRESULT hr;
   auto runOnMainThread = [&]() -> void {
     MOZ_ASSERT(NS_IsMainThread());
     hr = mTarget->QueryInterface(aIid, aOutput);
   };
   if (!invoker.Invoke(NS_NewRunnableFunction("Interceptor::QueryInterface", runOnMainThread))) {
     return E_FAIL;
@@ -770,23 +786,26 @@ Interceptor::WeakRefQueryInterface(REFII
   }
 
   if (aIid == IID_IMarshal) {
     MutexAutoLock lock(mStdMarshalMutex);
 
     HRESULT hr;
 
     if (!mStdMarshalUnk) {
+      MOZ_ASSERT(!tlsCreatingStdMarshal.get());
+      tlsCreatingStdMarshal.set(true);
       if (XRE_IsContentProcess()) {
         hr = FastMarshaler::Create(static_cast<IWeakReferenceSource*>(this),
                                    getter_AddRefs(mStdMarshalUnk));
       } else {
         hr = ::CoGetStdMarshalEx(static_cast<IWeakReferenceSource*>(this),
                                  SMEXF_SERVER, getter_AddRefs(mStdMarshalUnk));
       }
+      tlsCreatingStdMarshal.set(false);
 
       ENSURE_HR_SUCCEEDED(hr);
     }
 
     if (!mStdMarshal) {
       hr = mStdMarshalUnk->QueryInterface(IID_IMarshal, (void**)&mStdMarshal);
       ENSURE_HR_SUCCEEDED(hr);
 
--- a/ipc/mscom/Interceptor.h
+++ b/ipc/mscom/Interceptor.h
@@ -140,16 +140,17 @@ private:
   InterceptorTargetPtr<IUnknown>  mTarget;
   RefPtr<IInterceptorSink>  mEventSink;
   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
+  static MOZ_THREAD_LOCAL(bool) tlsCreatingStdMarshal;
 };
 
 template <typename InterfaceT>
 inline HRESULT
 CreateInterceptor(STAUniquePtr<InterfaceT> aTargetInterface,
                   IInterceptorSink* aEventSink,
                   InterfaceT** aOutInterface)
 {