Bug 1339951: Refactor mscom weak reference support and establish lock hierarchy within; r?jimm
MozReview-Commit-ID: BJJpSj44alY
--- a/ipc/mscom/Interceptor.cpp
+++ b/ipc/mscom/Interceptor.cpp
@@ -25,24 +25,25 @@ namespace mscom {
/* static */ HRESULT
Interceptor::Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
REFIID aIid, void** aOutput)
{
MOZ_ASSERT(aOutput && aTarget && aSink);
if (!aOutput) {
return E_INVALIDARG;
}
+
*aOutput = nullptr;
+
if (!aTarget || !aSink) {
return E_INVALIDARG;
}
- Interceptor* intcpt = new Interceptor(Move(aTarget), aSink);
- HRESULT hr = intcpt->QueryInterface(aIid, aOutput);
- static_cast<WeakReferenceSupport*>(intcpt)->Release();
- return hr;
+
+ RefPtr<WeakReferenceSupport> intcpt(new Interceptor(Move(aTarget), aSink));
+ return intcpt->QueryInterface(aIid, aOutput);
}
Interceptor::Interceptor(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink)
: WeakReferenceSupport(WeakReferenceSupport::Flags::eDestroyOnMainThread)
, mTarget(Move(aTarget))
, mEventSink(aSink)
, mMutex("mozilla::mscom::Interceptor::mMutex")
{
--- a/ipc/mscom/MainThreadHandoff.cpp
+++ b/ipc/mscom/MainThreadHandoff.cpp
@@ -48,25 +48,22 @@ private:
} // anonymous namespace
namespace mozilla {
namespace mscom {
/* static */ HRESULT
MainThreadHandoff::Create(IInterceptorSink** aOutput)
{
- *aOutput = nullptr;
- MainThreadHandoff* handoff = new MainThreadHandoff();
- HRESULT hr = handoff->QueryInterface(IID_IInterceptorSink, (void**) aOutput);
- handoff->Release();
- return hr;
+ RefPtr<MainThreadHandoff> handoff(new MainThreadHandoff());
+ return handoff->QueryInterface(IID_IInterceptorSink, (void**) aOutput);
}
MainThreadHandoff::MainThreadHandoff()
- : mRefCnt(1)
+ : mRefCnt(0)
{
}
MainThreadHandoff::~MainThreadHandoff()
{
MOZ_ASSERT(NS_IsMainThread());
}
--- a/ipc/mscom/WeakRef.cpp
+++ b/ipc/mscom/WeakRef.cpp
@@ -2,54 +2,117 @@
/* vim: set ts=8 sts=2 et sw=2 tw=80: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#define INITGUID
#include "mozilla/mscom/WeakRef.h"
-#include "mozilla/Assertions.h"
#include "mozilla/DebugOnly.h"
-#include "mozilla/RefPtr.h"
+#include "mozilla/Mutex.h"
#include "nsThreadUtils.h"
#include "nsWindowsHelpers.h"
+static void
+InitializeCS(CRITICAL_SECTION& aCS)
+{
+ DWORD flags = 0;
+#if defined(RELEASE_OR_BETA)
+ flags |= CRITICAL_SECTION_NO_DEBUG_INFO;
+#endif
+ InitializeCriticalSectionEx(&aCS, 4000, flags);
+}
+
namespace mozilla {
namespace mscom {
+namespace detail {
+
+SharedRef::SharedRef(WeakReferenceSupport* aSupport)
+ : mSupport(aSupport)
+{
+ ::InitializeCS(mCS);
+}
+
+SharedRef::~SharedRef()
+{
+ ::DeleteCriticalSection(&mCS);
+}
+
+void
+SharedRef::Lock()
+{
+ ::EnterCriticalSection(&mCS);
+}
+
+void
+SharedRef::Unlock()
+{
+ ::LeaveCriticalSection(&mCS);
+}
+
+HRESULT
+SharedRef::Resolve(REFIID aIid, void** aOutStrongReference)
+{
+ RefPtr<WeakReferenceSupport> strongRef;
+
+ { // Scope for lock
+ AutoCriticalSection lock(&mCS);
+ if (!mSupport) {
+ return E_POINTER;
+ }
+ strongRef = mSupport;
+ }
+
+ return strongRef->QueryInterface(aIid, aOutStrongReference);
+}
+
+void
+SharedRef::Clear()
+{
+ AutoCriticalSection lock(&mCS);
+ MOZ_ASSERT(mSupport);
+ mSupport = nullptr;
+}
+
+} // namespace detail
+
+typedef BaseAutoLock<detail::SharedRef> SharedRefAutoLock;
+typedef BaseAutoUnlock<detail::SharedRef> SharedRefAutoUnlock;
+
WeakReferenceSupport::WeakReferenceSupport(Flags aFlags)
- : mRefCnt(1)
+ : mRefCnt(0)
, mFlags(aFlags)
{
- ::InitializeCriticalSectionAndSpinCount(&mCS, 4000);
+ mSharedRef = new detail::SharedRef(this);
+ ::InitializeCS(mCSForQI);
}
WeakReferenceSupport::~WeakReferenceSupport()
{
- MOZ_ASSERT(mWeakRefs.IsEmpty());
- ::DeleteCriticalSection(&mCS);
+ ::DeleteCriticalSection(&mCSForQI);
}
HRESULT
WeakReferenceSupport::QueryInterface(REFIID riid, void** ppv)
{
- AutoCriticalSection lock(&mCS);
RefPtr<IUnknown> punk;
if (!ppv) {
return E_INVALIDARG;
}
*ppv = nullptr;
// Raise the refcount for stabilization purposes during aggregation
RefPtr<IUnknown> kungFuDeathGrip(this);
if (riid == IID_IUnknown || riid == IID_IWeakReferenceSource) {
punk = static_cast<IUnknown*>(this);
} else {
+ AutoCriticalSection lock(&mCSForQI);
HRESULT hr = ThreadSafeQueryInterface(riid, getter_AddRefs(punk));
if (FAILED(hr)) {
return hr;
}
}
if (!punk) {
return E_NOINTERFACE;
@@ -57,31 +120,34 @@ WeakReferenceSupport::QueryInterface(REF
punk.forget(ppv);
return S_OK;
}
ULONG
WeakReferenceSupport::AddRef()
{
- AutoCriticalSection lock(&mCS);
- return ++mRefCnt;
+ SharedRefAutoLock lock(*mSharedRef);
+ ULONG result = ++mRefCnt;
+ NS_LOG_ADDREF(this, result, "mscom::WeakReferenceSupport", sizeof(*this));
+ return result;
}
ULONG
WeakReferenceSupport::Release()
{
ULONG newRefCnt;
{ // Scope for lock
- AutoCriticalSection lock(&mCS);
+ SharedRefAutoLock lock(*mSharedRef);
newRefCnt = --mRefCnt;
if (newRefCnt == 0) {
- ClearWeakRefs();
+ mSharedRef->Clear();
}
}
+ NS_LOG_RELEASE(this, newRefCnt, "mscom::WeakReferenceSupport");
if (newRefCnt == 0) {
if (mFlags != Flags::eDestroyOnMainThread || NS_IsMainThread()) {
delete this;
} else {
// It is possible for the last Release() call to happen off-main-thread.
// If so, we need to dispatch an event to delete ourselves.
mozilla::DebugOnly<nsresult> rv =
NS_DispatchToMainThread(NS_NewRunnableFunction([this]() -> void
@@ -89,52 +155,32 @@ WeakReferenceSupport::Release()
delete this;
}));
MOZ_ASSERT(NS_SUCCEEDED(rv));
}
}
return newRefCnt;
}
-void
-WeakReferenceSupport::ClearWeakRefs()
-{
- for (uint32_t i = 0, len = mWeakRefs.Length(); i < len; ++i) {
- mWeakRefs[i]->Clear();
- mWeakRefs[i] = nullptr;
- }
- mWeakRefs.Clear();
-}
-
HRESULT
WeakReferenceSupport::GetWeakReference(IWeakReference** aOutWeakRef)
{
if (!aOutWeakRef) {
return E_INVALIDARG;
}
- *aOutWeakRef = nullptr;
- AutoCriticalSection lock(&mCS);
- RefPtr<WeakRef> weakRef = MakeAndAddRef<WeakRef>(this);
-
- HRESULT hr = weakRef->QueryInterface(IID_IWeakReference, (void**)aOutWeakRef);
- if (FAILED(hr)) {
- return hr;
- }
-
- mWeakRefs.AppendElement(weakRef);
- return S_OK;
+ RefPtr<WeakRef> weakRef = MakeAndAddRef<WeakRef>(mSharedRef);
+ return weakRef->QueryInterface(IID_IWeakReference, (void**)aOutWeakRef);
}
-WeakRef::WeakRef(WeakReferenceSupport* aSupport)
- : mRefCnt(1)
- , mMutex("mozilla::mscom::WeakRef::mMutex")
- , mSupport(aSupport)
+WeakRef::WeakRef(RefPtr<detail::SharedRef>& aSharedRef)
+ : mRefCnt(0)
+ , mSharedRef(aSharedRef)
{
- MOZ_ASSERT(aSupport);
+ MOZ_ASSERT(aSharedRef);
}
HRESULT
WeakRef::QueryInterface(REFIID riid, void** ppv)
{
IUnknown* punk = nullptr;
if (!ppv) {
return E_INVALIDARG;
@@ -151,42 +197,33 @@ WeakRef::QueryInterface(REFIID riid, voi
punk->AddRef();
return S_OK;
}
ULONG
WeakRef::AddRef()
{
- return (ULONG) InterlockedIncrement((LONG*)&mRefCnt);
+ ULONG result = ++mRefCnt;
+ NS_LOG_ADDREF(this, result, "mscom::WeakRef", sizeof(*this));
+ return result;
}
ULONG
WeakRef::Release()
{
- ULONG newRefCnt = (ULONG) InterlockedDecrement((LONG*)&mRefCnt);
+ ULONG newRefCnt = --mRefCnt;
+ NS_LOG_RELEASE(this, newRefCnt, "mscom::WeakRef");
if (newRefCnt == 0) {
delete this;
}
return newRefCnt;
}
HRESULT
WeakRef::Resolve(REFIID aIid, void** aOutStrongReference)
{
- MutexAutoLock lock(mMutex);
- if (!mSupport) {
- return E_FAIL;
- }
- return mSupport->QueryInterface(aIid, aOutStrongReference);
-}
-
-void
-WeakRef::Clear()
-{
- MutexAutoLock lock(mMutex);
- MOZ_ASSERT(mSupport);
- mSupport = nullptr;
+ return mSharedRef->Resolve(aIid, aOutStrongReference);
}
} // namespace mscom
} // namespace mozilla
--- a/ipc/mscom/WeakRef.h
+++ b/ipc/mscom/WeakRef.h
@@ -5,34 +5,67 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef mozilla_mscom_WeakRef_h
#define mozilla_mscom_WeakRef_h
#include <guiddef.h>
#include <unknwn.h>
-#include "mozilla/Mutex.h"
-#include "nsTArray.h"
+#include "mozilla/Assertions.h"
+#include "mozilla/Atomics.h"
+#include "mozilla/RefPtr.h"
+#include "nsISupportsImpl.h"
/**
* Thread-safe weak references for COM that works pre-Windows 8 and do not
* require WinRT.
*/
namespace mozilla {
namespace mscom {
+class WeakReferenceSupport;
+
+namespace detail {
+
+class SharedRef final
+{
+public:
+ explicit SharedRef(WeakReferenceSupport* aSupport);
+ void Lock();
+ void Unlock();
+
+ HRESULT Resolve(REFIID aIid, void** aOutStrongReference);
+ void Clear();
+
+ NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedRef)
+
+ SharedRef(const SharedRef&) = delete;
+ SharedRef(SharedRef&&) = delete;
+ SharedRef& operator=(const SharedRef&) = delete;
+ SharedRef& operator=(SharedRef&&) = delete;
+
+private:
+ ~SharedRef();
+
+private:
+ CRITICAL_SECTION mCS;
+ WeakReferenceSupport* mSupport;
+};
+
+} // namespace detail
+
// {F841AEFA-064C-49A4-B73D-EBD14A90F012}
DEFINE_GUID(IID_IWeakReference,
0xf841aefa, 0x64c, 0x49a4, 0xb7, 0x3d, 0xeb, 0xd1, 0x4a, 0x90, 0xf0, 0x12);
struct IWeakReference : public IUnknown
{
- virtual STDMETHODIMP Resolve(REFIID aIid, void** aOutStringReference) = 0;
+ virtual STDMETHODIMP Resolve(REFIID aIid, void** aOutStrongReference) = 0;
};
// {87611F0C-9BBB-4F78-9D43-CAC5AD432CA1}
DEFINE_GUID(IID_IWeakReferenceSource,
0x87611f0c, 0x9bbb, 0x4f78, 0x9d, 0x43, 0xca, 0xc5, 0xad, 0x43, 0x2c, 0xa1);
struct IWeakReferenceSource : public IUnknown
{
@@ -61,44 +94,37 @@ public:
protected:
explicit WeakReferenceSupport(Flags aFlags);
virtual ~WeakReferenceSupport();
virtual HRESULT ThreadSafeQueryInterface(REFIID aIid,
IUnknown** aOutInterface) = 0;
private:
- void ClearWeakRefs();
-
-private:
- // Using a raw CRITICAL_SECTION here because it can be reentered
- CRITICAL_SECTION mCS;
- ULONG mRefCnt;
- nsTArray<RefPtr<WeakRef>> mWeakRefs;
- Flags mFlags;
+ RefPtr<detail::SharedRef> mSharedRef;
+ ULONG mRefCnt;
+ Flags mFlags;
+ CRITICAL_SECTION mCSForQI;
};
class WeakRef : public IWeakReference
{
public:
// IUnknown
STDMETHODIMP QueryInterface(REFIID riid, void** ppv) override;
STDMETHODIMP_(ULONG) AddRef() override;
STDMETHODIMP_(ULONG) Release() override;
// IWeakReference
STDMETHODIMP Resolve(REFIID aIid, void** aOutStrongReference) override;
- explicit WeakRef(WeakReferenceSupport* aSupport);
-
- void Clear();
+ explicit WeakRef(RefPtr<detail::SharedRef>& aSharedRef);
private:
- ULONG mRefCnt;
- mozilla::Mutex mMutex; // Protects mSupport
- WeakReferenceSupport* mSupport;
+ Atomic<ULONG> mRefCnt;
+ RefPtr<detail::SharedRef> mSharedRef;
};
} // namespace mscom
} // namespace mozilla
#endif // mozilla_mscom_WeakRef_h