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
--- 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)
{