From 8d97cd82286641763018614500dbd13e05f4fbbb Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Mon, 18 Oct 2021 22:59:17 +0000 Subject: [PATCH] Bug 1725572 - Part 1: Support automatic cleanup for ManagedEndpoint instances, r=handyman This patch adds support for ManagedEndpoint instances to be dropped & gracefully destroyed. Before this change, a ManagedEndpoint which was dropped without being bound would not clean up its' peer actor, meaning that messages to and from that actor would be discarded. This is done by adding a new actor destroy reason for dropping a ManagedEndpoint. Differential Revision: https://phabricator.services.mozilla.com/D128776 --- ipc/glue/Endpoint.cpp | 148 ++++++++++++++++++++++++++++++++ ipc/glue/Endpoint.h | 89 +++++++++++++------ ipc/glue/ProtocolMessageUtils.h | 55 +++++++----- ipc/glue/ProtocolUtils.cpp | 4 + ipc/glue/ProtocolUtils.h | 18 ++-- ipc/glue/moz.build | 1 + ipc/ipdl/ipdl/lower.py | 72 +++++++++++++--- 7 files changed, 318 insertions(+), 69 deletions(-) create mode 100644 ipc/glue/Endpoint.cpp diff --git a/ipc/glue/Endpoint.cpp b/ipc/glue/Endpoint.cpp new file mode 100644 index 000000000000..e6fa5727a7af --- /dev/null +++ b/ipc/glue/Endpoint.cpp @@ -0,0 +1,148 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* 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/. */ + +#include "mozilla/ipc/Endpoint.h" +#include "chrome/common/ipc_message.h" +#include "mozilla/ipc/IPDLParamTraits.h" +#include "nsThreadUtils.h" +#include "mozilla/ipc/ProtocolMessageUtils.h" + +namespace mozilla::ipc { + +UntypedManagedEndpoint::UntypedManagedEndpoint(IProtocol* aActor) + : mInner(Some(Inner{ + /* mOtherSide */ aActor->GetWeakLifecycleProxy(), + /* mToplevel */ nullptr, + aActor->Id(), + aActor->GetProtocolId(), + aActor->Manager()->Id(), + aActor->Manager()->GetProtocolId(), + })) {} + +UntypedManagedEndpoint::~UntypedManagedEndpoint() { + if (!IsValid()) { + return; + } + + if (mInner->mOtherSide) { + // If this ManagedEndpoint was never sent over IPC, deliver a fake + // MANAGED_ENDPOINT_DROPPED_MESSAGE_TYPE message directly to the other side + // actor. + mInner->mOtherSide->ActorEventTarget()->Dispatch(NS_NewRunnableFunction( + "~ManagedEndpoint (Local)", + [otherSide = mInner->mOtherSide, id = mInner->mId] { + if (IProtocol* actor = otherSide->Get(); actor && actor->CanRecv()) { + MOZ_DIAGNOSTIC_ASSERT(actor->Id() == id, "Wrong Actor?"); + RefPtr strongProxy(actor->GetLifecycleProxy()); + strongProxy->Get()->OnMessageReceived( + IPC::Message(id, MANAGED_ENDPOINT_DROPPED_MESSAGE_TYPE)); + } + })); + } else if (mInner->mToplevel) { + // If it was sent over IPC, we'll need to send the message to the sending + // side. Let's send the message async. + mInner->mToplevel->ActorEventTarget()->Dispatch(NS_NewRunnableFunction( + "~ManagedEndpoint (Remote)", + [toplevel = mInner->mToplevel, id = mInner->mId] { + if (IProtocol* actor = toplevel->Get(); + actor && actor->CanSend() && actor->GetIPCChannel()) { + actor->GetIPCChannel()->Send(MakeUnique( + id, MANAGED_ENDPOINT_DROPPED_MESSAGE_TYPE)); + } + })); + } +} + +bool UntypedManagedEndpoint::BindCommon(IProtocol* aActor, + IProtocol* aManager) { + MOZ_ASSERT(aManager); + if (!mInner) { + NS_WARNING("Cannot bind to invalid endpoint"); + return false; + } + + // Perform thread assertions. + if (mInner->mToplevel) { + MOZ_DIAGNOSTIC_ASSERT( + mInner->mToplevel->ActorEventTarget()->IsOnCurrentThread()); + MOZ_DIAGNOSTIC_ASSERT(aManager->ToplevelProtocol() == + mInner->mToplevel->Get()); + } + + if (NS_WARN_IF(aManager->Id() != mInner->mManagerId) || + NS_WARN_IF(aManager->GetProtocolId() != mInner->mManagerType) || + NS_WARN_IF(aActor->GetProtocolId() != mInner->mType)) { + MOZ_ASSERT_UNREACHABLE("Actor and manager do not match Endpoint"); + return false; + } + + if (!aManager->CanSend() || !aManager->GetIPCChannel()) { + NS_WARNING("Manager cannot send"); + return false; + } + + int32_t id = mInner->mId; + mInner.reset(); + + // Our typed caller will insert the actor into the managed container. + aActor->SetManagerAndRegister(aManager, id); + + aManager->GetIPCChannel()->Send( + MakeUnique(id, MANAGED_ENDPOINT_BOUND_MESSAGE_TYPE)); + return true; +} + +/* static */ +void IPDLParamTraits::Write(IPC::Message* aMsg, + IProtocol* aActor, + paramType&& aParam) { + bool isValid = aParam.mInner.isSome(); + WriteIPDLParam(aMsg, aActor, isValid); + if (!isValid) { + return; + } + + auto inner = std::move(*aParam.mInner); + aParam.mInner.reset(); + + MOZ_RELEASE_ASSERT(inner.mOtherSide, "Has not been sent over IPC yet"); + MOZ_RELEASE_ASSERT(inner.mOtherSide->ActorEventTarget()->IsOnCurrentThread(), + "Must be being sent from the correct thread"); + MOZ_RELEASE_ASSERT( + inner.mOtherSide->Get() && inner.mOtherSide->Get()->ToplevelProtocol() == + aActor->ToplevelProtocol(), + "Must be being sent over the same toplevel protocol"); + + WriteIPDLParam(aMsg, aActor, inner.mId); + WriteIPDLParam(aMsg, aActor, inner.mType); + WriteIPDLParam(aMsg, aActor, inner.mManagerId); + WriteIPDLParam(aMsg, aActor, inner.mManagerType); +} + +/* static */ +bool IPDLParamTraits::Read(const IPC::Message* aMsg, + PickleIterator* aIter, + IProtocol* aActor, + paramType* aResult) { + *aResult = UntypedManagedEndpoint{}; + bool isValid = false; + if (!aActor || !ReadIPDLParam(aMsg, aIter, aActor, &isValid)) { + return false; + } + if (!isValid) { + return true; + } + + aResult->mInner.emplace(); + auto& inner = *aResult->mInner; + inner.mToplevel = aActor->ToplevelProtocol()->GetWeakLifecycleProxy(); + return ReadIPDLParam(aMsg, aIter, aActor, &inner.mId) && + ReadIPDLParam(aMsg, aIter, aActor, &inner.mType) && + ReadIPDLParam(aMsg, aIter, aActor, &inner.mManagerId) && + ReadIPDLParam(aMsg, aIter, aActor, &inner.mManagerType); +} + +} // namespace mozilla::ipc diff --git a/ipc/glue/Endpoint.h b/ipc/glue/Endpoint.h index 09fbb54a3bc3..8ac37e8b75fb 100644 --- a/ipc/glue/Endpoint.h +++ b/ipc/glue/Endpoint.h @@ -15,6 +15,7 @@ #include "mozilla/Maybe.h" #include "mozilla/UniquePtr.h" #include "mozilla/ipc/MessageLink.h" +#include "mozilla/ipc/ProtocolUtils.h" #include "mozilla/ipc/Transport.h" #include "mozilla/ipc/NodeController.h" #include "mozilla/ipc/ScopedPort.h" @@ -118,6 +119,51 @@ nsresult CreateEndpoints(const PrivateIPDLInterface& aPrivate, return NS_OK; } +class UntypedManagedEndpoint { + public: + bool IsValid() const { return mInner.isSome(); } + + UntypedManagedEndpoint(const UntypedManagedEndpoint&) = delete; + UntypedManagedEndpoint& operator=(const UntypedManagedEndpoint&) = delete; + + protected: + UntypedManagedEndpoint() = default; + explicit UntypedManagedEndpoint(IProtocol* aActor); + + UntypedManagedEndpoint(UntypedManagedEndpoint&& aOther) noexcept + : mInner(std::move(aOther.mInner)) { + aOther.mInner = Nothing(); + } + UntypedManagedEndpoint& operator=(UntypedManagedEndpoint&& aOther) noexcept { + this->~UntypedManagedEndpoint(); + new (this) UntypedManagedEndpoint(std::move(aOther)); + return *this; + } + + ~UntypedManagedEndpoint() noexcept; + + bool BindCommon(IProtocol* aActor, IProtocol* aManager); + + private: + friend struct IPDLParamTraits; + + struct Inner { + // Pointers to the toplevel actor which will manage this connection. When + // created, only `mOtherSide` will be set, and will reference the + // toplevel actor which the other side is managed by. After being sent over + // IPC, only `mToplevel` will be set, and will be the toplevel actor for the + // channel which received the IPC message. + RefPtr mOtherSide; + RefPtr mToplevel; + + int32_t mId = 0; + ProtocolId mType = LastMsgIndex; + int32_t mManagerId = 0; + ProtocolId mManagerType = LastMsgIndex; + }; + Maybe mInner; +}; + /** * A managed endpoint represents one end of a partially initialized managed * IPDL actor. It is used for situations where the usual IPDL Constructor @@ -140,39 +186,28 @@ nsresult CreateEndpoints(const PrivateIPDLInterface& aPrivate, * a browser crash. */ template -class ManagedEndpoint { +class ManagedEndpoint : public UntypedManagedEndpoint { public: - ManagedEndpoint() : mId(0) {} + ManagedEndpoint() = default; + ManagedEndpoint(ManagedEndpoint&&) noexcept = default; + ManagedEndpoint& operator=(ManagedEndpoint&&) noexcept = default; - ManagedEndpoint(const PrivateIPDLInterface&, int32_t aId) : mId(aId) {} + ManagedEndpoint(const PrivateIPDLInterface&, IProtocol* aActor) + : UntypedManagedEndpoint(aActor) {} - ManagedEndpoint(ManagedEndpoint&& aOther) : mId(aOther.mId) { - aOther.mId = 0; + bool Bind(const PrivateIPDLInterface&, PFooSide* aActor, IProtocol* aManager, + ManagedContainer& aContainer) { + if (!BindCommon(aActor, aManager)) { + return false; + } + aContainer.Insert(aActor); + return true; } - ManagedEndpoint& operator=(ManagedEndpoint&& aOther) { - mId = aOther.mId; - aOther.mId = 0; - return *this; + // Only invalid ManagedEndpoints can be equal, as valid endpoints are unique. + bool operator==(const ManagedEndpoint& _o) const { + return !IsValid() && !_o.IsValid(); } - - bool IsValid() const { return mId != 0; } - - Maybe ActorId() const { return IsValid() ? Some(mId) : Nothing(); } - - bool operator==(const ManagedEndpoint& _o) const { return mId == _o.mId; } - - private: - friend struct IPC::ParamTraits>; - - ManagedEndpoint(const ManagedEndpoint&) = delete; - ManagedEndpoint& operator=(const ManagedEndpoint&) = delete; - - // The routing ID for the to-be-created endpoint. - int32_t mId; - - // XXX(nika): Might be nice to have other info for assertions? - // e.g. mManagerId, mManagerType, etc. }; } // namespace ipc diff --git a/ipc/glue/ProtocolMessageUtils.h b/ipc/glue/ProtocolMessageUtils.h index 887460eb9837..c8e9b4fa1e2c 100644 --- a/ipc/glue/ProtocolMessageUtils.h +++ b/ipc/glue/ProtocolMessageUtils.h @@ -14,6 +14,7 @@ #include "chrome/common/ipc_message_utils.h" #include "ipc/EnumSerializer.h" #include "mozilla/Assertions.h" +#include "mozilla/ipc/Endpoint.h" #include "mozilla/ipc/ProtocolUtils.h" #include "mozilla/ipc/Transport.h" @@ -38,6 +39,11 @@ struct ParamTraits : ContiguousEnumSerializerInclusive {}; +template <> +struct ParamTraits + : ContiguousEnumSerializer {}; + template <> struct ParamTraits { typedef mozilla::ipc::ActorHandle paramType; @@ -83,32 +89,35 @@ struct ParamTraits> { } }; -template -struct ParamTraits> { - typedef mozilla::ipc::ManagedEndpoint paramType; - - static void Write(Message* aMsg, const paramType& aParam) { - IPC::WriteParam(aMsg, aParam.mId); - } - - static bool Read(const Message* aMsg, PickleIterator* aIter, - paramType* aResult) { - MOZ_RELEASE_ASSERT(aResult->mId == 0); - - if (!IPC::ReadParam(aMsg, aIter, &aResult->mId)) { - return false; - } - return true; - } - - static void Log(const paramType& aParam, std::wstring* aLog) { - aLog->append(StringPrintf(L"ManagedEndpoint")); - } -}; - } // namespace IPC namespace mozilla::ipc { + +template <> +struct IPDLParamTraits { + using paramType = UntypedManagedEndpoint; + + static void Write(IPC::Message* aMsg, IProtocol* aActor, paramType&& aParam); + static bool Read(const IPC::Message* aMsg, PickleIterator* aIter, + IProtocol* aActor, paramType* aResult); +}; + +template +struct IPDLParamTraits> { + using paramType = ManagedEndpoint; + + static void Write(IPC::Message* aMsg, IProtocol* aActor, paramType&& aParam) { + IPDLParamTraits::Write(aMsg, aActor, + std::move(aParam)); + } + + static bool Read(const IPC::Message* aMsg, PickleIterator* aIter, + IProtocol* aActor, paramType* aResult) { + return IPDLParamTraits::Read(aMsg, aIter, aActor, + aResult); + } +}; + template <> struct IPDLParamTraits { typedef FileDescriptor paramType; diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp index ea88763c9af2..68a33e9bedf7 100644 --- a/ipc/glue/ProtocolUtils.cpp +++ b/ipc/glue/ProtocolUtils.cpp @@ -293,6 +293,10 @@ IProtocol* WeakActorLifecycleProxy::Get() const { return mProxy ? mProxy->Get() : nullptr; } +WeakActorLifecycleProxy* IProtocol::GetWeakLifecycleProxy() { + return mLifecycleProxy ? mLifecycleProxy->GetWeakProxy() : nullptr; +} + IProtocol::~IProtocol() { // If the actor still has a lifecycle proxy when it is being torn down, it // means that IPC was not given control over the lifecycle of the actor diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index 3fee03991776..ae562f26a092 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -59,13 +59,15 @@ namespace { // starts approaching its 65,536th message. enum { // Message types used by NodeChannel - ACCEPT_INVITE_MESSAGE_TYPE = kuint16max - 14, - REQUEST_INTRODUCTION_MESSAGE_TYPE = kuint16max - 13, - INTRODUCE_MESSAGE_TYPE = kuint16max - 12, - BROADCAST_MESSAGE_TYPE = kuint16max - 11, - EVENT_MESSAGE_TYPE = kuint16max - 10, + ACCEPT_INVITE_MESSAGE_TYPE = kuint16max - 16, + REQUEST_INTRODUCTION_MESSAGE_TYPE = kuint16max - 15, + INTRODUCE_MESSAGE_TYPE = kuint16max - 14, + BROADCAST_MESSAGE_TYPE = kuint16max - 13, + EVENT_MESSAGE_TYPE = kuint16max - 12, // Message types used by MessageChannel + MANAGED_ENDPOINT_DROPPED_MESSAGE_TYPE = kuint16max - 11, + MANAGED_ENDPOINT_BOUND_MESSAGE_TYPE = kuint16max - 10, IMPENDING_SHUTDOWN_MESSAGE_TYPE = kuint16max - 9, BUILD_IDS_MATCH_MESSAGE_TYPE = kuint16max - 8, BUILD_ID_MESSAGE_TYPE = kuint16max - 7, // unused @@ -175,6 +177,7 @@ class IToplevelProtocol; class ActorLifecycleProxy; class WeakActorLifecycleProxy; class IPDLResolverInner; +class UntypedManagedEndpoint; class IProtocol : public HasResultCodes { public: @@ -183,7 +186,8 @@ class IProtocol : public HasResultCodes { Deletion, AncestorDeletion, NormalShutdown, - AbnormalShutdown + AbnormalShutdown, + ManagedEndpointDropped }; typedef base::ProcessId ProcessId; @@ -246,6 +250,7 @@ class IProtocol : public HasResultCodes { IProtocol* Manager() const { return mManager; } ActorLifecycleProxy* GetLifecycleProxy() { return mLifecycleProxy; } + WeakActorLifecycleProxy* GetWeakLifecycleProxy(); Side GetSide() const { return mSide; } bool CanSend() const { return mLinkStatus == LinkStatus::Connected; } @@ -283,6 +288,7 @@ class IProtocol : public HasResultCodes { friend class IToplevelProtocol; friend class ActorLifecycleProxy; friend class IPDLResolverInner; + friend class UntypedManagedEndpoint; void SetId(int32_t aId); diff --git a/ipc/glue/moz.build b/ipc/glue/moz.build index fc8e043455f8..0ab54d97c87c 100644 --- a/ipc/glue/moz.build +++ b/ipc/glue/moz.build @@ -161,6 +161,7 @@ UNIFIED_SOURCES += [ "BrowserProcessSubThread.cpp", "CrashReporterClient.cpp", "CrashReporterHost.cpp", + "Endpoint.cpp", "FileDescriptor.cpp", "FileDescriptorUtils.cpp", "IdleSchedulerChild.cpp", diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py index e40cbcf46353..5bc316119ee4 100644 --- a/ipc/ipdl/ipdl/lower.py +++ b/ipc/ipdl/ipdl/lower.py @@ -467,6 +467,7 @@ class _DestroyReason: NormalShutdown = ExprVar("NormalShutdown") AbnormalShutdown = ExprVar("AbnormalShutdown") FailedConstructor = ExprVar("FailedConstructor") + ManagedEndpointDropped = ExprVar("ManagedEndpointDropped") class _ResponseRejectReason: @@ -4004,6 +4005,40 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): if toplevel.isInterrupt(): self.interruptSwitch = StmtSwitch(msgtype) + # Add a handler for the MANAGED_ENDPOINT_BOUND and + # MANAGED_ENDPOINT_DROPPED message types for managed actors. + if not ptype.isToplevel(): + clearawaitingmanagedendpointbind = """ + if (!mAwaitingManagedEndpointBind) { + NS_WARNING("Unexpected managed endpoint lifecycle message after actor bound!"); + return MsgNotAllowed; + } + mAwaitingManagedEndpointBind = false; + """ + self.asyncSwitch.addcase( + CaseLabel("MANAGED_ENDPOINT_BOUND_MESSAGE_TYPE"), + StmtBlock( + [ + StmtCode(clearawaitingmanagedendpointbind), + StmtReturn(_Result.Processed), + ] + ), + ) + self.asyncSwitch.addcase( + CaseLabel("MANAGED_ENDPOINT_DROPPED_MESSAGE_TYPE"), + StmtBlock( + [ + StmtCode(clearawaitingmanagedendpointbind), + *self.destroyActor( + None, + ExprVar.THIS, + why=_DestroyReason.ManagedEndpointDropped, + ), + StmtReturn(_Result.Processed), + ] + ), + ) + # implement Send*() methods and add dispatcher cases to # message switch()es for md in p.messageDecls: @@ -4199,6 +4234,17 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): # or we're toplevel self.cls.addstmts([clearsubtree, Whitespace.NL]) + if not ptype.isToplevel(): + self.cls.addstmts( + [ + StmtDecl( + Decl(Type.BOOL, "mAwaitingManagedEndpointBind"), + init=ExprLiteral.FALSE, + ), + Whitespace.NL, + ] + ) + for managed in ptype.manages: self.cls.addstmts( [ @@ -4232,7 +4278,11 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): openmeth.addcode( """ $*{bind} - return ${thereEp}(mozilla::ipc::PrivateIPDLInterface(), aActor->Id()); + // Mark our actor as awaiting the other side to be bound. This will + // be cleared when a `MANAGED_ENDPOINT_{DROPPED,BOUND}` message is + // received. + aActor->mAwaitingManagedEndpointBind = true; + return ${thereEp}(mozilla::ipc::PrivateIPDLInterface(), aActor); """, bind=self.bindManagedActor(actor, errfn=ExprCall(ExprVar(thereEp))), thereEp=thereEp, @@ -4251,13 +4301,9 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): ) bindmeth.addcode( """ - MOZ_RELEASE_ASSERT(aEndpoint.ActorId(), "Invalid Endpoint!"); - $*{bind} - return true; + return aEndpoint.Bind(mozilla::ipc::PrivateIPDLInterface(), aActor, this, ${container}); """, - bind=self.bindManagedActor( - actor, errfn=ExprLiteral.FALSE, idexpr=ExprCode("*aEndpoint.ActorId()") - ), + container=self.protocol.managedVar(managed, self.side), ) self.cls.addstmts([openmeth, bindmeth, Whitespace.NL]) @@ -4749,7 +4795,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): return method def destroyActor(self, md, actorexpr, why=_DestroyReason.Deletion): - if md.decl.type.isCtor(): + if md and md.decl.type.isCtor(): destroyedType = md.decl.type.constructedType() else: destroyedType = self.protocol.decl.type @@ -4757,11 +4803,11 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): return [ StmtCode( """ - IProtocol* mgr = ${actor}->Manager(); - ${actor}->DestroySubtree(${why}); - ${actor}->ClearSubtree(); - mgr->RemoveManagee(${protoId}, ${actor}); - """, + IProtocol* mgr = ${actor}->Manager(); + ${actor}->DestroySubtree(${why}); + ${actor}->ClearSubtree(); + mgr->RemoveManagee(${protoId}, ${actor}); + """, actor=actorexpr, why=why, protoId=_protocolId(destroyedType),