Bug 1434822 part 2: mscom: Add a function to disconnect all remote clients associated with a given target. r?aklotz
Because Interceptors disable COM garbage collection to improve performance, they never receive Release calls from remote clients.
If the object can be shut down while clients still hold a reference, this function can be used to force COM to disconnect all remote connections (using CoDisconnectObject) and thus release the associated references to the Interceptor, its target and any objects associated with the HandlerProvider.
A HandlerProvider::DisconnectHandlerRemotes method also had to be added to allow HandlerProviders to disconnect clients for their own objects.
MozReview-Commit-ID: JaxEkOtrP1M
--- a/ipc/mscom/IHandlerProvider.h
+++ b/ipc/mscom/IHandlerProvider.h
@@ -18,16 +18,17 @@ namespace mscom {
struct IInterceptor;
struct HandlerProvider
{
virtual STDMETHODIMP GetHandler(NotNull<CLSID*> aHandlerClsid) = 0;
virtual STDMETHODIMP GetHandlerPayloadSize(NotNull<IInterceptor*> aInterceptor, NotNull<DWORD*> aOutPayloadSize) = 0;
virtual STDMETHODIMP WriteHandlerPayload(NotNull<IInterceptor*> aInterceptor, NotNull<IStream*> aStream) = 0;
virtual STDMETHODIMP_(REFIID) MarshalAs(REFIID aIid) = 0;
+ virtual STDMETHODIMP DisconnectHandlerRemotes() = 0;
};
struct IHandlerProvider : public IUnknown
, public HandlerProvider
{
virtual STDMETHODIMP_(REFIID) GetEffectiveOutParamIid(REFIID aCallIid,
ULONG aCallMethod) = 0;
virtual STDMETHODIMP NewInstance(REFIID aIid,
--- a/ipc/mscom/Interceptor.cpp
+++ b/ipc/mscom/Interceptor.cpp
@@ -423,16 +423,17 @@ HRESULT
Interceptor::ReleaseMarshalData(IStream* pStm)
{
return mStdMarshal->ReleaseMarshalData(pStm);
}
HRESULT
Interceptor::DisconnectObject(DWORD dwReserved)
{
+ mEventSink->DisconnectHandlerRemotes();
return mStdMarshal->DisconnectObject(dwReserved);
}
Interceptor::MapEntry*
Interceptor::Lookup(REFIID aIid)
{
mInterceptorMapMutex.AssertCurrentThreadOwns();
@@ -825,10 +826,35 @@ Interceptor::AddRef()
}
ULONG
Interceptor::Release()
{
return WeakReferenceSupport::Release();
}
+/* static */ HRESULT
+Interceptor::DisconnectRemotesForTarget(IUnknown* aTarget)
+{
+ MOZ_ASSERT(aTarget);
+
+ detail::LiveSetAutoLock lock(GetLiveSet());
+
+ // It is not an error if the interceptor doesn't exist, so we return
+ // S_FALSE instead of an error in that case.
+ RefPtr<IWeakReference> existingWeak(Move(GetLiveSet().Get(aTarget)));
+ if (!existingWeak) {
+ return S_FALSE;
+ }
+
+ RefPtr<IWeakReferenceSource> existingStrong;
+ if (FAILED(existingWeak->ToStrongRef(getter_AddRefs(existingStrong)))) {
+ return S_FALSE;
+ }
+ // Since we now hold a strong ref on the interceptor, we may now release the
+ // lock.
+ lock.Unlock();
+
+ return ::CoDisconnectObject(existingStrong, 0);
+}
+
} // namespace mscom
} // namespace mozilla
--- a/ipc/mscom/Interceptor.h
+++ b/ipc/mscom/Interceptor.h
@@ -69,16 +69,36 @@ class Interceptor final : public WeakRef
, public IStdMarshalInfo
, public IMarshal
, public IInterceptor
{
public:
static HRESULT Create(STAUniquePtr<IUnknown> aTarget, IInterceptorSink* aSink,
REFIID aInitialIid, void** aOutInterface);
+ /**
+ * Disconnect all remote clients for a given target.
+ * Because Interceptors disable COM garbage collection to improve
+ * performance, they never receive Release calls from remote clients. If
+ * the object can be shut down while clients still hold a reference, this
+ * function can be used to force COM to disconnect all remote connections
+ * (using CoDisconnectObject) and thus release the associated references to
+ * the Interceptor, its target and any objects associated with the
+ * HandlerProvider.
+ * Note that the specified target must be the same IUnknown pointer used to
+ * create the Interceptor. Where there is multiple inheritance, querying for
+ * IID_IUnknown and calling this function with that pointer alone will not
+ * disconnect remotes for all interfaces. If you expect that the same object
+ * may be fetched with different initial interfaces, you should call this
+ * function once for each possible IUnknown pointer.
+ * @return S_OK if there was an Interceptor for the given target,
+ * S_FALSE if there was not.
+ */
+ static HRESULT DisconnectRemotesForTarget(IUnknown* aTarget);
+
// IUnknown
STDMETHODIMP QueryInterface(REFIID riid, void** ppv) override;
STDMETHODIMP_(ULONG) AddRef() override;
STDMETHODIMP_(ULONG) Release() override;
// IStdMarshalInfo
STDMETHODIMP GetClassForHandler(DWORD aDestContext, void* aDestContextPtr,
CLSID* aHandlerClsid) override;
--- a/ipc/mscom/MainThreadHandoff.cpp
+++ b/ipc/mscom/MainThreadHandoff.cpp
@@ -585,16 +585,26 @@ MainThreadHandoff::MarshalAs(REFIID aIid
{
if (!mHandlerProvider) {
return aIid;
}
return mHandlerProvider->MarshalAs(aIid);
}
HRESULT
+MainThreadHandoff::DisconnectHandlerRemotes()
+{
+ if (!mHandlerProvider) {
+ return E_NOTIMPL;
+ }
+
+ return mHandlerProvider->DisconnectHandlerRemotes();
+}
+
+HRESULT
MainThreadHandoff::OnWalkInterface(REFIID aIid, PVOID* aInterface,
BOOL aIsInParam, BOOL aIsOutParam)
{
MOZ_ASSERT(aInterface && aIsOutParam);
if (!aInterface || !aIsOutParam) {
return E_UNEXPECTED;
}
--- a/ipc/mscom/MainThreadHandoff.h
+++ b/ipc/mscom/MainThreadHandoff.h
@@ -61,16 +61,17 @@ public:
// IInterceptorSink
STDMETHODIMP SetInterceptor(IWeakReference* aInterceptor) override;
STDMETHODIMP GetHandler(NotNull<CLSID*> aHandlerClsid) override;
STDMETHODIMP GetHandlerPayloadSize(NotNull<IInterceptor*> aInterceptor,
NotNull<DWORD*> aOutPayloadSize) override;
STDMETHODIMP WriteHandlerPayload(NotNull<IInterceptor*> aInterceptor,
NotNull<IStream*> aStream) override;
STDMETHODIMP_(REFIID) MarshalAs(REFIID aIid) override;
+ STDMETHODIMP DisconnectHandlerRemotes() override;
// ICallFrameWalker
STDMETHODIMP OnWalkInterface(REFIID aIid, PVOID* aInterface, BOOL aIsInParam,
BOOL aIsOutParam) override;
private:
explicit MainThreadHandoff(IHandlerProvider* aHandlerProvider);
~MainThreadHandoff();