From 59203c63bb777a3bacec32fb8830fba33540e809 Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Wed, 12 Jun 2024 07:35:29 +0200 Subject: Adding upstream version 127.0. Signed-off-by: Daniel Baumann --- ipc/chromium/gtest/moz.build | 7 +- ipc/chromium/gtest/ports_unittest.cc | 4 + ipc/chromium/src/base/revocable_store.cc | 4 +- ipc/chromium/src/mojo/core/ports/node.cc | 9 + ipc/chromium/src/mojo/core/ports/node_delegate.h | 6 + ipc/glue/BackgroundImpl.cpp | 7 +- ipc/glue/Endpoint.cpp | 4 +- ipc/glue/Endpoint.h | 5 +- ipc/glue/IPCMessageUtilsSpecializations.h | 4 + ipc/glue/MessageChannel.cpp | 8 +- ipc/glue/NodeController.cpp | 132 ++--- ipc/glue/NodeController.h | 6 + ipc/glue/ProtocolUtils.cpp | 220 +++++---- ipc/glue/ProtocolUtils.h | 109 +++-- ipc/ipdl/Makefile.in | 2 + ipc/ipdl/ipdl.py | 585 +++++++++++++---------- ipc/ipdl/ipdl/__init__.py | 2 +- ipc/ipdl/ipdl/lower.py | 179 ++----- ipc/ipdl/test/gtest/PTestDestroyNested.ipdl | 18 + ipc/ipdl/test/gtest/PTestDestroyNestedSub.ipdl | 19 + ipc/ipdl/test/gtest/TestDestroyNested.cpp | 123 +++++ ipc/ipdl/test/gtest/moz.build | 3 + 22 files changed, 850 insertions(+), 606 deletions(-) create mode 100644 ipc/ipdl/test/gtest/PTestDestroyNested.ipdl create mode 100644 ipc/ipdl/test/gtest/PTestDestroyNestedSub.ipdl create mode 100644 ipc/ipdl/test/gtest/TestDestroyNested.cpp (limited to 'ipc') diff --git a/ipc/chromium/gtest/moz.build b/ipc/chromium/gtest/moz.build index afa175c638..a1af974b16 100644 --- a/ipc/chromium/gtest/moz.build +++ b/ipc/chromium/gtest/moz.build @@ -8,9 +8,14 @@ Library("ipcchromiumtest") SOURCES += [ "name_unittest.cc", - "ports_unittest.cc", ] +# Bug 1837550 - Fails under TSAN +if not CONFIG["MOZ_TSAN"]: + SOURCES += [ + "ports_unittest.cc", + ] + include("/ipc/chromium/chromium-config.mozbuild") FINAL_LIBRARY = "xul-gtest" diff --git a/ipc/chromium/gtest/ports_unittest.cc b/ipc/chromium/gtest/ports_unittest.cc index bfa8214799..705bab7edb 100644 --- a/ipc/chromium/gtest/ports_unittest.cc +++ b/ipc/chromium/gtest/ports_unittest.cc @@ -237,6 +237,10 @@ class TestNode : public NodeDelegate { } } + void ObserveRemoteNode(const NodeName& node) override { + DCHECK(node != node_name_); + } + void ClosePortsInEvent(Event* event) { if (event->type() != Event::Type::kUserMessage) { return; diff --git a/ipc/chromium/src/base/revocable_store.cc b/ipc/chromium/src/base/revocable_store.cc index b3d7414ee2..0b8a0655bd 100644 --- a/ipc/chromium/src/base/revocable_store.cc +++ b/ipc/chromium/src/base/revocable_store.cc @@ -25,9 +25,7 @@ RevocableStore::~RevocableStore() { owning_reference_->set_store(NULL); } -void RevocableStore::Add(Revocable* item) { - DCHECK(!item->revoked()); -} +void RevocableStore::Add(Revocable* item) { DCHECK(!item->revoked()); } void RevocableStore::RevokeAll() { // We revoke all the existing items in the store and reset our count. diff --git a/ipc/chromium/src/mojo/core/ports/node.cc b/ipc/chromium/src/mojo/core/ports/node.cc index 935771eeb7..036c981cbb 100644 --- a/ipc/chromium/src/mojo/core/ports/node.cc +++ b/ipc/chromium/src/mojo/core/ports/node.cc @@ -837,6 +837,10 @@ int Node::OnObserveProxy(const PortRef& port_ref, MaybeResendAckRequest(port_ref); delegate_->PortStatusChanged(port_ref); + + if (event->proxy_target_node_name() != name_) { + delegate_->ObserveRemoteNode(event->proxy_target_node_name()); + } } return OK; @@ -1513,6 +1517,11 @@ int Node::AcceptPort(const PortName& port_name, mozilla::MakeUnique( port_descriptor.referring_port_name, kInvalidPortName, kInvalidSequenceNum)); + + if (port_descriptor.peer_node_name != name_) { + delegate_->ObserveRemoteNode(port_descriptor.peer_node_name); + } + return OK; } diff --git a/ipc/chromium/src/mojo/core/ports/node_delegate.h b/ipc/chromium/src/mojo/core/ports/node_delegate.h index 3172779c06..37fb67613a 100644 --- a/ipc/chromium/src/mojo/core/ports/node_delegate.h +++ b/ipc/chromium/src/mojo/core/ports/node_delegate.h @@ -29,6 +29,12 @@ class NodeDelegate { // to query the latest status of the port. Note, this event could be spurious // if another thread is simultaneously modifying the status of the port. virtual void PortStatusChanged(const PortRef& port_ref) = 0; + + // Called after receiving a port with a remote peer, or bypassing a proxy to a + // remote peer. Embedders can use this to ensure a connection to the remote + // peer, reducing message queueing and ensuring prompt notification of peer + // node death. + virtual void ObserveRemoteNode(const NodeName& node) = 0; }; } // namespace ports diff --git a/ipc/glue/BackgroundImpl.cpp b/ipc/glue/BackgroundImpl.cpp index bba09c261a..bf040a18a4 100644 --- a/ipc/glue/BackgroundImpl.cpp +++ b/ipc/glue/BackgroundImpl.cpp @@ -561,7 +561,12 @@ class ChildImpl final : public BackgroundChildImpl { #endif } - NS_INLINE_DECL_REFCOUNTING(ChildImpl, override) + // This type is threadsafe refcounted as actors managed by it may be destroyed + // after the thread it is bound to dies, and hold a reference to this object. + // + // It is _not_ safe to use this type or any methods on it from off of the + // thread it was created for. + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ChildImpl, override) private: // Forwarded from BackgroundChild. diff --git a/ipc/glue/Endpoint.cpp b/ipc/glue/Endpoint.cpp index 3391f8b359..728c37fa3f 100644 --- a/ipc/glue/Endpoint.cpp +++ b/ipc/glue/Endpoint.cpp @@ -57,7 +57,7 @@ UntypedManagedEndpoint::~UntypedManagedEndpoint() { } bool UntypedManagedEndpoint::BindCommon(IProtocol* aActor, - IProtocol* aManager) { + IRefCountedProtocol* aManager) { MOZ_ASSERT(aManager); if (!mInner) { NS_WARNING("Cannot bind to invalid endpoint"); @@ -88,7 +88,7 @@ bool UntypedManagedEndpoint::BindCommon(IProtocol* aActor, mInner.reset(); // Our typed caller will insert the actor into the managed container. - aActor->SetManagerAndRegister(aManager, id); + MOZ_ALWAYS_TRUE(aActor->SetManagerAndRegister(aManager, id)); aManager->GetIPCChannel()->Send( MakeUnique(id, MANAGED_ENDPOINT_BOUND_MESSAGE_TYPE)); diff --git a/ipc/glue/Endpoint.h b/ipc/glue/Endpoint.h index d7eea94dfc..e4d32a7119 100644 --- a/ipc/glue/Endpoint.h +++ b/ipc/glue/Endpoint.h @@ -214,7 +214,7 @@ class UntypedManagedEndpoint { ~UntypedManagedEndpoint() noexcept; - bool BindCommon(IProtocol* aActor, IProtocol* aManager); + bool BindCommon(IProtocol* aActor, IRefCountedProtocol* aManager); private: friend struct IPDLParamTraits; @@ -267,7 +267,8 @@ class ManagedEndpoint : public UntypedManagedEndpoint { ManagedEndpoint(const PrivateIPDLInterface&, IProtocol* aActor) : UntypedManagedEndpoint(aActor) {} - bool Bind(const PrivateIPDLInterface&, PFooSide* aActor, IProtocol* aManager, + bool Bind(const PrivateIPDLInterface&, PFooSide* aActor, + IRefCountedProtocol* aManager, ManagedContainer& aContainer) { if (!BindCommon(aActor, aManager)) { return false; diff --git a/ipc/glue/IPCMessageUtilsSpecializations.h b/ipc/glue/IPCMessageUtilsSpecializations.h index e9742c5c74..ec0620d057 100644 --- a/ipc/glue/IPCMessageUtilsSpecializations.h +++ b/ipc/glue/IPCMessageUtilsSpecializations.h @@ -775,6 +775,7 @@ struct ParamTraits { WriteParam(aWriter, aParam.mSizes); WriteParam(aWriter, aParam.mType); WriteParam(aWriter, aParam.mMedia); + WriteParam(aWriter, aParam.mAnchor); WriteParam(aWriter, aParam.mCrossOrigin); WriteParam(aWriter, aParam.mReferrerPolicy); WriteParam(aWriter, aParam.mAs); @@ -812,6 +813,9 @@ struct ParamTraits { if (!ReadParam(aReader, &aResult->mMedia)) { return false; } + if (!ReadParam(aReader, &aResult->mAnchor)) { + return false; + } if (!ReadParam(aReader, &aResult->mCrossOrigin)) { return false; } diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index 16eb897175..544ce59b8e 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -1604,9 +1604,13 @@ nsresult MessageChannel::MessageTask::Run() { return NS_OK; } + Channel()->AssertWorkerThread(); + mMonitor->AssertSameMonitor(*Channel()->mMonitor); + #ifdef FUZZING_SNAPSHOT if (!mIsFuzzMsg) { - if (fuzzing::Nyx::instance().started()) { + if (fuzzing::Nyx::instance().started() && XRE_IsParentProcess() && + Channel()->IsCrossProcess()) { // Once we started fuzzing, prevent non-fuzzing tasks from being // run and potentially blocking worker threads. // @@ -1622,8 +1626,6 @@ nsresult MessageChannel::MessageTask::Run() { } #endif - Channel()->AssertWorkerThread(); - mMonitor->AssertSameMonitor(*Channel()->mMonitor); proxy = Channel()->Listener()->GetLifecycleProxy(); Channel()->RunMessage(proxy, *this); diff --git a/ipc/glue/NodeController.cpp b/ipc/glue/NodeController.cpp index 7781771660..325eff1834 100644 --- a/ipc/glue/NodeController.cpp +++ b/ipc/glue/NodeController.cpp @@ -307,67 +307,78 @@ void NodeController::DropPeer(NodeName aNodeName) { mNode->LostConnectionToNode(aNodeName); } -void NodeController::ForwardEvent(const NodeName& aNode, - UniquePtr aEvent) { - if (aNode == mName) { - (void)mNode->AcceptEvent(mName, std::move(aEvent)); - } else { - // On Windows and macOS, messages holding HANDLEs or mach ports must be - // relayed via the broker process so it can transfer ownership. - bool needsRelay = false; +void NodeController::ContactRemotePeer(const NodeName& aNode, + UniquePtr aEvent) { + // On Windows and macOS, messages holding HANDLEs or mach ports must be + // relayed via the broker process so it can transfer ownership. + bool needsRelay = false; #if defined(XP_WIN) || defined(XP_DARWIN) - if (!IsBroker() && aNode != kBrokerNodeName && - aEvent->type() == Event::kUserMessage) { - auto* userEvent = static_cast(aEvent.get()); - needsRelay = - userEvent->HasMessage() && - userEvent->GetMessage()->num_relayed_attachments() > 0; - } + if (aEvent && !IsBroker() && aNode != kBrokerNodeName && + aEvent->type() == Event::kUserMessage) { + auto* userEvent = static_cast(aEvent.get()); + needsRelay = + userEvent->HasMessage() && + userEvent->GetMessage()->num_relayed_attachments() > 0; + } #endif - UniquePtr message = + UniquePtr message; + if (aEvent) { + message = SerializeEventMessage(std::move(aEvent), needsRelay ? &aNode : nullptr); MOZ_ASSERT(message->is_relay() == needsRelay, "Message relay status set incorrectly"); + } - RefPtr peer; - RefPtr broker; - bool needsIntroduction = false; - { - auto state = mState.Lock(); + RefPtr peer; + RefPtr broker; + bool needsIntroduction = false; + bool needsBroker = needsRelay; + { + auto state = mState.Lock(); - // Check if we know this peer. If we don't, we'll need to request an - // introduction. - peer = state->mPeers.Get(aNode); - if (!peer || needsRelay) { - if (IsBroker()) { - NODECONTROLLER_WARNING("Ignoring message '%s' to unknown peer %s", - message->name(), ToString(aNode).c_str()); - return; - } - - broker = state->mPeers.Get(kBrokerNodeName); - if (!broker) { - NODECONTROLLER_WARNING( - "Ignoring message '%s' to peer %s due to a missing broker", - message->name(), ToString(aNode).c_str()); - return; - } - - if (!needsRelay) { - auto& queue = - state->mPendingMessages.LookupOrInsertWith(aNode, [&]() { - needsIntroduction = true; - return Queue, 64>{}; - }); - queue.Push(std::move(message)); - } + // Check if we know this peer. If we don't, we'll need to request an + // introduction. + peer = state->mPeers.Get(aNode); + if (!peer) { + // We don't know the peer, check if we've already requested an + // introduction, or if we need to request a new one. + auto& queue = state->mPendingMessages.LookupOrInsertWith(aNode, [&]() { + needsIntroduction = true; + needsBroker = true; + return Queue, 64>{}; + }); + // If we aren't relaying, queue up the message to be sent. + if (message && !needsRelay) { + queue.Push(std::move(message)); } } - MOZ_ASSERT(!needsIntroduction || !needsRelay, - "Only one of the two should ever be set"); + if (needsBroker && !IsBroker()) { + broker = state->mPeers.Get(kBrokerNodeName); + } + } + if (needsBroker && !broker) { + NODECONTROLLER_WARNING( + "Dropping message '%s'; no connection to unknown peer %s", + message ? message->name() : "", ToString(aNode).c_str()); + if (needsIntroduction) { + // We have no broker and will never be able to be introduced to this node. + // Queue a task to clean up any ports connected to it. + XRE_GetIOMessageLoop()->PostTask(NewRunnableMethod( + "NodeController::DropPeer", this, &NodeController::DropPeer, aNode)); + } + return; + } + + if (needsIntroduction) { + NODECONTROLLER_LOG(LogLevel::Info, "Requesting introduction to peer %s", + ToString(aNode).c_str()); + broker->RequestIntroduction(aNode); + } + + if (message) { if (needsRelay) { NODECONTROLLER_LOG(LogLevel::Info, "Relaying message '%s' for peer %s due to %" PRIu32 @@ -376,15 +387,22 @@ void NodeController::ForwardEvent(const NodeName& aNode, message->num_relayed_attachments()); MOZ_ASSERT(message->num_relayed_attachments() > 0 && broker); broker->SendEventMessage(std::move(message)); - } else if (needsIntroduction) { - MOZ_ASSERT(broker); - broker->RequestIntroduction(aNode); } else if (peer) { peer->SendEventMessage(std::move(message)); } } } +void NodeController::ForwardEvent(const NodeName& aNode, + UniquePtr aEvent) { + MOZ_ASSERT(aEvent, "cannot forward null event"); + if (aNode == mName) { + (void)mNode->AcceptEvent(mName, std::move(aEvent)); + } else { + ContactRemotePeer(aNode, std::move(aEvent)); + } +} + void NodeController::BroadcastEvent(UniquePtr aEvent) { UniquePtr message = SerializeEventMessage(std::move(aEvent), nullptr, BROADCAST_MESSAGE_TYPE); @@ -415,6 +433,11 @@ void NodeController::PortStatusChanged(const PortRef& aPortRef) { } } +void NodeController::ObserveRemoteNode(const NodeName& aNode) { + MOZ_ASSERT(aNode != mName); + ContactRemotePeer(aNode, nullptr); +} + void NodeController::OnEventMessage(const NodeName& aFromNode, UniquePtr aMessage) { AssertIOThread(); @@ -715,8 +738,6 @@ void NodeController::OnAcceptInvite(const NodeName& aFromNode, Invite invite; { auto state = mState.Lock(); - MOZ_ASSERT(state->mPendingMessages.IsEmpty(), - "Shouldn't have pending messages in broker"); // Try to remove the source node from our invites list and insert it into // our peers map under the new name. @@ -840,6 +861,9 @@ void NodeController::CleanUp() { lostConnections.AppendElement(chan.GetKey()); channelsToClose.AppendElement(chan.GetData()); } + for (const auto& pending : state->mPendingMessages.Keys()) { + lostConnections.AppendElement(pending); + } for (const auto& invite : state->mInvites.Values()) { channelsToClose.AppendElement(invite.mChannel); portsToClose.AppendElement(invite.mToMerge); diff --git a/ipc/glue/NodeController.h b/ipc/glue/NodeController.h index 5356b85084..c1c2d33750 100644 --- a/ipc/glue/NodeController.h +++ b/ipc/glue/NodeController.h @@ -121,6 +121,11 @@ class NodeController final : public mojo::core::ports::NodeDelegate, // Stop communicating with this peer. Must be called on the IO thread. void DropPeer(NodeName aNodeName); + // Ensure that there is a direct connection to a remote node, requesting an + // introduction if there is not. + // If provided, will optionally send an event to the remote node. + void ContactRemotePeer(const NodeName& aNode, UniquePtr aEvent); + // Message Handlers void OnEventMessage(const NodeName& aFromNode, UniquePtr aMessage) override; @@ -138,6 +143,7 @@ class NodeController final : public mojo::core::ports::NodeDelegate, void ForwardEvent(const NodeName& aNode, UniquePtr aEvent) override; void BroadcastEvent(UniquePtr aEvent) override; void PortStatusChanged(const PortRef& aPortRef) override; + void ObserveRemoteNode(const NodeName& aNode) override; const NodeName mName; const UniquePtr mNode; diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp index b29221d476..42bab98320 100644 --- a/ipc/glue/ProtocolUtils.cpp +++ b/ipc/glue/ProtocolUtils.cpp @@ -251,12 +251,6 @@ ActorLifecycleProxy::ActorLifecycleProxy(IProtocol* aActor) : mActor(aActor) { MOZ_ASSERT(mActor->CanSend(), "Cannot create LifecycleProxy for non-connected actor!"); - // Take a reference to our manager's lifecycle proxy to try to hold it & - // ensure it doesn't die before us. - if (mActor->mManager) { - mManager = mActor->mManager->mLifecycleProxy; - } - // Record that we've taken our first reference to our actor. mActor->ActorAlloc(); } @@ -321,6 +315,7 @@ IProtocol::~IProtocol() { // Gecko, simply emit a warning and clear the weak backreference from our // LifecycleProxy back to us. if (mLifecycleProxy) { + MOZ_ASSERT(mLinkStatus != LinkStatus::Inactive); NS_WARNING( nsPrintfCString("Actor destructor for '%s%s' called before IPC " "lifecycle complete!\n" @@ -329,34 +324,13 @@ IProtocol::~IProtocol() { .get()); mLifecycleProxy->mActor = nullptr; - - // If we are somehow being destroyed while active, make sure that the - // existing IPC reference has been freed. If the status of the actor is - // `Destroyed`, the reference has already been freed, and we shouldn't free - // it a second time. - MOZ_ASSERT(mLinkStatus != LinkStatus::Inactive); - if (mLinkStatus != LinkStatus::Destroyed) { - NS_IF_RELEASE(mLifecycleProxy); - } mLifecycleProxy = nullptr; } } // The following methods either directly forward to the toplevel protocol, or // almost directly do. -int32_t IProtocol::Register(IProtocol* aRouted) { - return mToplevel->Register(aRouted); -} -int32_t IProtocol::RegisterID(IProtocol* aRouted, int32_t aId) { - return mToplevel->RegisterID(aRouted, aId); -} IProtocol* IProtocol::Lookup(int32_t aId) { return mToplevel->Lookup(aId); } -void IProtocol::Unregister(int32_t aId) { - if (aId == mId) { - mId = kFreedActorId; - } - return mToplevel->Unregister(aId); -} Shmem::SharedMemory* IProtocol::CreateSharedMemory(size_t aSize, bool aUnsafe, int32_t* aId) { @@ -383,11 +357,6 @@ nsISerialEventTarget* IProtocol::GetActorEventTarget() { return GetIPCChannel()->GetWorkerEventTarget(); } -void IProtocol::SetId(int32_t aId) { - MOZ_ASSERT(mId == aId || mLinkStatus == LinkStatus::Inactive); - mId = aId; -} - Maybe IProtocol::ReadActor(IPC::MessageReader* aReader, bool aNullable, const char* aActorDescription, @@ -487,26 +456,60 @@ bool IProtocol::DeallocShmem(Shmem& aMem) { return ok; } -void IProtocol::SetManager(IProtocol* aManager) { +void IProtocol::SetManager(IRefCountedProtocol* aManager) { MOZ_RELEASE_ASSERT(!mManager || mManager == aManager); mManager = aManager; mToplevel = aManager->mToplevel; } -void IProtocol::SetManagerAndRegister(IProtocol* aManager) { - // Set the manager prior to registering so registering properly inherits - // the manager's event target. - SetManager(aManager); +bool IProtocol::SetManagerAndRegister(IRefCountedProtocol* aManager, + int32_t aId) { + MOZ_RELEASE_ASSERT(mLinkStatus == LinkStatus::Inactive, + "Actor must be inactive to SetManagerAndRegister"); - aManager->Register(this); -} + // Set to `false` if the actor is to be torn down after registration. + bool success = true; -void IProtocol::SetManagerAndRegister(IProtocol* aManager, int32_t aId) { // Set the manager prior to registering so registering properly inherits // the manager's event target. SetManager(aManager); - aManager->RegisterID(this, aId); + mId = aId == kNullActorId ? mToplevel->NextId() : aId; + while (mToplevel->mActorMap.Contains(mId)) { + // The ID already existing is an error case, but we want to proceed with + // registration so that we can tear down the actor cleanly - generate a new + // ID for that case. + NS_WARNING("Actor already exists with the selected ID!"); + mId = mToplevel->NextId(); + success = false; + } + + RefPtr proxy = ActorConnected(); + mToplevel->mActorMap.InsertOrUpdate(mId, proxy); + MOZ_ASSERT(proxy->Get() == this); + + // If our manager is already dying, mark ourselves as doomed as well. + if (aManager && aManager->mLinkStatus != LinkStatus::Connected) { + mLinkStatus = LinkStatus::Doomed; + if (aManager->mLinkStatus != LinkStatus::Doomed) { + // Our manager is already fully dead, make sure we call + // `ActorDisconnected`. + success = false; + } + } + + // If setting the manager failed, call `ActorDisconnected` and return false. + if (!success) { + ActorDisconnected(FailedConstructor); + MOZ_ASSERT(mLinkStatus == LinkStatus::Destroyed); + return false; + } + return true; +} + +void IProtocol::UnlinkManager() { + mToplevel = nullptr; + mManager = nullptr; } bool IProtocol::ChannelSend(UniquePtr aMsg) { @@ -540,9 +543,9 @@ void IProtocol::WarnMessageDiscarded(IPC::Message* aMsg) { } #endif -void IProtocol::ActorConnected() { +already_AddRefed IProtocol::ActorConnected() { if (mLinkStatus != LinkStatus::Inactive) { - return; + return nullptr; } #ifdef FUZZING_SNAPSHOT @@ -552,73 +555,81 @@ void IProtocol::ActorConnected() { mLinkStatus = LinkStatus::Connected; MOZ_ASSERT(!mLifecycleProxy, "double-connecting live actor"); - mLifecycleProxy = new ActorLifecycleProxy(this); - NS_ADDREF(mLifecycleProxy); // Reference freed in DestroySubtree(); + RefPtr proxy = new ActorLifecycleProxy(this); + mLifecycleProxy = proxy; + return proxy.forget(); } -void IProtocol::DoomSubtree() { - MOZ_ASSERT(CanSend(), "dooming non-connected actor"); - MOZ_ASSERT(mLifecycleProxy, "dooming zombie actor"); - - nsTArray> managed; - AllManagedActors(managed); - for (ActorLifecycleProxy* proxy : managed) { - // Guard against actor being disconnected or destroyed during previous Doom - IProtocol* actor = proxy->Get(); - if (actor && actor->CanSend()) { - actor->DoomSubtree(); - } +void IProtocol::ActorDisconnected(ActorDestroyReason aWhy) { + MOZ_ASSERT(mLifecycleProxy, "destroying zombie actor"); + // If the actor has already been marked as `Destroyed`, there's nothing to do. + if (mLinkStatus != LinkStatus::Connected && + mLinkStatus != LinkStatus::Doomed) { + return; } - // ActorDoom is called immediately before changing state, this allows messages - // to be sent during ActorDoom immediately before the channel is closed and - // sending messages is disabled. - ActorDoom(); - mLinkStatus = LinkStatus::Doomed; -} + // Mark the entire subtree as doomed so that no further messages can be + // sent/recieved, and newly created managed actors are immediately marked as + // doomed on creation. + DoomSubtree(); -void IProtocol::DestroySubtree(ActorDestroyReason aWhy) { - MOZ_ASSERT(CanRecv(), "destroying non-connected actor"); - MOZ_ASSERT(mLifecycleProxy, "destroying zombie actor"); + // Perform the steps to fully destroy an actor after it has been unregistered + // from its manager. + auto doActorDestroy = [toplevel = mToplevel, ipcChannel = GetIPCChannel()]( + IProtocol* actor, ActorDestroyReason why) { + MOZ_ASSERT(actor->mLinkStatus == LinkStatus::Doomed, + "Actor must be doomed when calling doActorDestroy"); + MOZ_ASSERT(actor->AllManagedActorsCount() == 0, + "All managed actors must have been destroyed first"); + + // Mark the actor as Destroyed, ensuring we can't re-enter `ActorDestroy`, + // even if an callback spins a nested event loop. + actor->mLinkStatus = LinkStatus::Destroyed; #ifdef FUZZING_SNAPSHOT - fuzzing::IPCFuzzController::instance().OnActorDestroyed(this); + fuzzing::IPCFuzzController::instance().OnActorDestroyed(actor); #endif - int32_t id = Id(); + int32_t id = actor->mId; + if (IProtocol* manager = actor->Manager()) { + actor->mId = kFreedActorId; + auto entry = toplevel->mActorMap.Lookup(id); + MOZ_DIAGNOSTIC_ASSERT(entry && *entry == actor->GetLifecycleProxy(), + "ID must be present and reference this actor"); + entry.Remove(); + manager->RemoveManagee(actor->GetProtocolId(), actor); + } + + ipcChannel->RejectPendingResponsesForActor(id); + actor->ActorDestroy(why); + }; - // If we're a managed actor, unregister from our manager - if (Manager()) { - Unregister(id); - } + // Hold all ActorLifecycleProxy instances for managed actors until we return. + nsTArray> proxyHolder; + proxyHolder.AppendElement(GetLifecycleProxy()); - // Destroy subtree + // Invoke `ActorDestroy` for all managed actors in the subtree. These are + // handled one at a time, so that new actors which are potentially registered + // during `ActorDestroy` callbacks are not missed. ActorDestroyReason subtreeWhy = aWhy; if (aWhy == Deletion || aWhy == FailedConstructor) { subtreeWhy = AncestorDeletion; } - - nsTArray> managed; - AllManagedActors(managed); - for (ActorLifecycleProxy* proxy : managed) { - // Guard against actor being disconnected or destroyed during previous - // Destroy - IProtocol* actor = proxy->Get(); - if (actor && actor->CanRecv()) { - actor->DestroySubtree(subtreeWhy); + while (IProtocol* actor = PeekManagedActor()) { + // If the selected actor manages other actors, destroy those first. + while (IProtocol* inner = actor->PeekManagedActor()) { + actor = inner; } - } - // Ensure that we don't send any messages while we're calling `ActorDestroy` - // by setting our state to `Doomed`. - mLinkStatus = LinkStatus::Doomed; + proxyHolder.AppendElement(actor->GetLifecycleProxy()); + doActorDestroy(actor, subtreeWhy); + } - // The actor is being destroyed, reject any pending responses, invoke - // `ActorDestroy` to destroy it, and then clear our status to - // `LinkStatus::Destroyed`. - GetIPCChannel()->RejectPendingResponsesForActor(id); - ActorDestroy(aWhy); - mLinkStatus = LinkStatus::Destroyed; + // Destroy ourselves if we were not not otherwise destroyed while destroying + // managed actors. + if (mLinkStatus == LinkStatus::Doomed) { + doActorDestroy(this, aWhy); + } } IToplevelProtocol::IToplevelProtocol(const char* aName, ProtocolId aProtoId, @@ -689,28 +700,11 @@ int32_t IToplevelProtocol::NextId() { return (++mLastLocalId << 2) | tag; } -int32_t IToplevelProtocol::Register(IProtocol* aRouted) { - if (aRouted->Id() != kNullActorId && aRouted->Id() != kFreedActorId) { - // If there's already an ID, just return that. - return aRouted->Id(); +IProtocol* IToplevelProtocol::Lookup(int32_t aId) { + if (auto entry = mActorMap.Lookup(aId)) { + return entry.Data()->Get(); } - return RegisterID(aRouted, NextId()); -} - -int32_t IToplevelProtocol::RegisterID(IProtocol* aRouted, int32_t aId) { - aRouted->SetId(aId); - aRouted->ActorConnected(); - MOZ_ASSERT(!mActorMap.Contains(aId), "Don't insert with an existing ID"); - mActorMap.InsertOrUpdate(aId, aRouted); - return aId; -} - -IProtocol* IToplevelProtocol::Lookup(int32_t aId) { return mActorMap.Get(aId); } - -void IToplevelProtocol::Unregister(int32_t aId) { - MOZ_ASSERT(mActorMap.Contains(aId), - "Attempting to remove an ID not in the actor map"); - mActorMap.Remove(aId); + return nullptr; } Shmem::SharedMemory* IToplevelProtocol::CreateSharedMemory(size_t aSize, diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index 2800a41c91..995ef2f256 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -124,12 +124,16 @@ enum class LinkStatus : uint8_t { // A live link is connected to the other side of this actor. Connected, - // The link has begun being destroyed. Messages may still be received, but - // cannot be sent. (exception: sync replies may be sent while Doomed). + // The link has begun being destroyed. Messages may no longer be sent. The + // ActorDestroy method is queued to be called, but has not been invoked yet, + // as managed actors still need to be destroyed first. + // + // NOTE: While no new IPC can be received at this point, `CanRecv` will still + // be true until `LinkStatus::Destroyed`. Doomed, - // The link has been destroyed, and messages will no longer be sent or - // received. + // The actor has been destroyed, and ActorDestroy has been called, however an + // ActorLifecycleProxy still holds a reference to the actor. Destroyed, }; @@ -171,12 +175,8 @@ class IProtocol : public HasResultCodes { IToplevelProtocol* ToplevelProtocol() { return mToplevel; } const IToplevelProtocol* ToplevelProtocol() const { return mToplevel; } - // The following methods either directly forward to the toplevel protocol, or - // almost directly do. - int32_t Register(IProtocol* aRouted); - int32_t RegisterID(IProtocol* aRouted, int32_t aId); + // Lookup() is forwarded directly to the toplevel protocol. IProtocol* Lookup(int32_t aId); - void Unregister(int32_t aId); Shmem::SharedMemory* CreateSharedMemory(size_t aSize, bool aUnsafe, int32_t* aId); @@ -197,13 +197,16 @@ class IProtocol : public HasResultCodes { const char* GetProtocolName() const { return ProtocolIdToName(mProtocolId); } int32_t Id() const { return mId; } - IProtocol* Manager() const { return mManager; } + IRefCountedProtocol* Manager() const { return mManager; } ActorLifecycleProxy* GetLifecycleProxy() { return mLifecycleProxy; } WeakActorLifecycleProxy* GetWeakLifecycleProxy(); Side GetSide() const { return mSide; } bool CanSend() const { return mLinkStatus == LinkStatus::Connected; } + + // Returns `true` for an active actor until the actor's `ActorDestroy` method + // has been called. bool CanRecv() const { return mLinkStatus == LinkStatus::Connected || mLinkStatus == LinkStatus::Doomed; @@ -235,17 +238,20 @@ class IProtocol : public HasResultCodes { friend class IPDLResolverInner; friend class UntypedManagedEndpoint; - void SetId(int32_t aId); + // We have separate functions because the accessibility code and BrowserParent + // manually calls SetManager. + void SetManager(IRefCountedProtocol* aManager); - // We have separate functions because the accessibility code manually - // calls SetManager. - void SetManager(IProtocol* aManager); + // Clear `mManager` and `mToplevel` to nullptr. Only intended to be called + // within the unlink implementation of cycle collected IPDL actors with cycle + // collected managers. + void UnlinkManager(); // Sets the manager for the protocol and registers the protocol with // its manager, setting up channels for the protocol as well. Not // for use outside of IPDL. - void SetManagerAndRegister(IProtocol* aManager); - void SetManagerAndRegister(IProtocol* aManager, int32_t aId); + bool SetManagerAndRegister(IRefCountedProtocol* aManager, + int32_t aId = kNullActorId); // Helpers for calling `Send` on our underlying IPC channel. bool ChannelSend(UniquePtr aMsg); @@ -265,28 +271,31 @@ class IProtocol : public HasResultCodes { } } - // Collect all actors managed by this object in an array. To make this safer - // to iterate, `ActorLifecycleProxy` references are returned rather than raw - // actor pointers. - virtual void AllManagedActors( - nsTArray>& aActors) const = 0; - virtual uint32_t AllManagedActorsCount() const = 0; // Internal method called when the actor becomes connected. - void ActorConnected(); + already_AddRefed ActorConnected(); + + // Internal method called when actor becomes disconnected. + void ActorDisconnected(ActorDestroyReason aWhy); + + // Called by DoomSubtree on each managed actor to mark it as Doomed and + // prevent further IPC. + void SetDoomed() { + MOZ_ASSERT(mLinkStatus == LinkStatus::Connected || + mLinkStatus == LinkStatus::Doomed, + "Invalid link status for SetDoomed"); + mLinkStatus = LinkStatus::Doomed; + } + virtual void DoomSubtree() = 0; - // Called immediately before setting the actor state to doomed, and triggering - // async actor destruction. Messages may be sent from this callback, but no - // later. - // FIXME(nika): This is currently unused! - virtual void ActorDoom() {} - void DoomSubtree(); + // Internal function returning an arbitrary directly managed actor. Used to + // identify managed actors to destroy when tearing down an actor tree. + virtual IProtocol* PeekManagedActor() = 0; // Called when the actor has been destroyed due to an error, a __delete__ // message, or a __doom__ reply. virtual void ActorDestroy(ActorDestroyReason aWhy) {} - void DestroySubtree(ActorDestroyReason aWhy); // Called when IPC has acquired its first reference to the actor. This method // may take references which will later be freed by `ActorDealloc`. @@ -313,7 +322,7 @@ class IProtocol : public HasResultCodes { Side mSide; LinkStatus mLinkStatus; ActorLifecycleProxy* mLifecycleProxy; - IProtocol* mManager; + RefPtr mManager; IToplevelProtocol* mToplevel; }; @@ -414,6 +423,7 @@ class IRefCountedProtocol : public IProtocol { * this protocol actor. */ class IToplevelProtocol : public IRefCountedProtocol { + friend class IProtocol; template friend class Endpoint; @@ -423,12 +433,8 @@ class IToplevelProtocol : public IRefCountedProtocol { ~IToplevelProtocol() = default; public: - // Shadow methods on IProtocol which are implemented directly on toplevel - // actors. - int32_t Register(IProtocol* aRouted); - int32_t RegisterID(IProtocol* aRouted, int32_t aId); + // Shadows the method on IProtocol, which will forward to the top. IProtocol* Lookup(int32_t aId); - void Unregister(int32_t aId); Shmem::SharedMemory* CreateSharedMemory(size_t aSize, bool aUnsafe, int32_t* aId); @@ -441,8 +447,6 @@ class IToplevelProtocol : public IRefCountedProtocol { void SetOtherProcessId(base::ProcessId aOtherPid); - virtual void OnChannelClose() = 0; - virtual void OnChannelError() = 0; virtual void ProcessingError(Result aError, const char* aMsgName) {} bool Open(ScopedPort aPort, const nsID& aMessageChannelId, @@ -508,7 +512,26 @@ class IToplevelProtocol : public IRefCountedProtocol { virtual void OnChannelReceivedMessage(const Message& aMsg) {} - void OnIPCChannelOpened() { ActorConnected(); } + // MessageChannel lifecycle callbacks. + void OnIPCChannelOpened() { + // Leak the returned ActorLifecycleProxy reference. It will be destroyed in + // `OnChannelClose` or `OnChannelError`. + Unused << ActorConnected(); + } + void OnChannelClose() { + // Re-acquire the ActorLifecycleProxy reference acquired in + // OnIPCChannelOpened. + RefPtr proxy = dont_AddRef(GetLifecycleProxy()); + ActorDisconnected(NormalShutdown); + DeallocShmems(); + } + void OnChannelError() { + // Re-acquire the ActorLifecycleProxy reference acquired in + // OnIPCChannelOpened. + RefPtr proxy = dont_AddRef(GetLifecycleProxy()); + ActorDisconnected(AbnormalShutdown); + DeallocShmems(); + } base::ProcessId OtherPidMaybeInvalid() const { return mOtherPid; } @@ -523,7 +546,7 @@ class IToplevelProtocol : public IRefCountedProtocol { // NOTE NOTE NOTE // Used to be on mState int32_t mLastLocalId; - IDMap mActorMap; + IDMap> mActorMap; IDMap> mShmemMap; MessageChannel mChannel; @@ -652,10 +675,6 @@ class ActorLifecycleProxy { IProtocol* MOZ_NON_OWNING_REF mActor; - // Hold a reference to the actor's manager's ActorLifecycleProxy to help - // prevent it from dying while we're still alive! - RefPtr mManager; - // When requested, the current self-referencing weak reference for this // ActorLifecycleProxy. RefPtr mWeakProxy; @@ -753,6 +772,8 @@ class ManagedContainer { } } + Protocol* Peek() { return mArray.IsEmpty() ? nullptr : mArray.LastElement(); } + void Clear() { mArray.Clear(); } private: diff --git a/ipc/ipdl/Makefile.in b/ipc/ipdl/Makefile.in index 6ebff9cba0..4847569a56 100644 --- a/ipc/ipdl/Makefile.in +++ b/ipc/ipdl/Makefile.in @@ -19,6 +19,7 @@ ipdl_py_deps := \ # NB: the IPDL compiler manages .ipdl-->.h/.cpp dependencies itself, # which is why we don't have explicit .h/.cpp targets here ipdl.track: $(ALL_IPDLSRCS) $(ALL_IPDLSRCS_FILE) $(srcdir)/sync-messages.ini $(srcdir)/message-metadata.ini $(ipdl_py_deps) + $(call BUILDSTATUS,START_Ipdl ipdl.py) $(PYTHON3) $(srcdir)/ipdl.py \ --sync-msg-list=$(srcdir)/sync-messages.ini \ --msg-metadata=$(srcdir)/message-metadata.ini \ @@ -27,6 +28,7 @@ ipdl.track: $(ALL_IPDLSRCS) $(ALL_IPDLSRCS_FILE) $(srcdir)/sync-messages.ini $(s $(IPDLDIRS:%=-I%) \ --file-list $(ALL_IPDLSRCS_FILE) touch $@ + $(call BUILDSTATUS,END_Ipdl ipdl.py) export:: ipdl.track endif diff --git a/ipc/ipdl/ipdl.py b/ipc/ipdl/ipdl.py index 8e5dd5db3d..5c4b7a8344 100644 --- a/ipc/ipdl/ipdl.py +++ b/ipc/ipdl/ipdl.py @@ -6,214 +6,291 @@ import os import sys from configparser import RawConfigParser from io import StringIO +from multiprocessing import Manager import ipdl from ipdl.ast import SYNC -def log(minv, fmt, *args): - if _verbosity >= minv: - print(fmt % args) - - -# process command line - - -op = optparse.OptionParser(usage="ipdl.py [options] IPDLfiles...") -op.add_option( - "-I", - "--include", - dest="includedirs", - default=[], - action="append", - help="Additional directory to search for included protocol specifications", -) -op.add_option( - "-s", - "--sync-msg-list", - dest="syncMsgList", - default="sync-messages.ini", - help="Config file listing allowed sync messages", -) -op.add_option( - "-m", - "--msg-metadata", - dest="msgMetadata", - default="message-metadata.ini", - help="Predicted message sizes for reducing serialization malloc overhead.", -) -op.add_option( - "-v", - "--verbose", - dest="verbosity", - default=1, - action="count", - help="Verbose logging (specify -vv or -vvv for very verbose logging)", -) -op.add_option( - "-q", - "--quiet", - dest="verbosity", - action="store_const", - const=0, - help="Suppress logging output", -) -op.add_option( - "-d", - "--outheaders-dir", - dest="headersdir", - default=".", - help="""Directory into which C++ headers will be generated. -A protocol Foo in the namespace bar will cause the headers - dir/bar/Foo.h, dir/bar/FooParent.h, and dir/bar/FooParent.h -to be generated""", -) -op.add_option( - "-o", - "--outcpp-dir", - dest="cppdir", - default=".", - help="""Directory into which C++ sources will be generated -A protocol Foo in the namespace bar will cause the sources - cppdir/FooParent.cpp, cppdir/FooChild.cpp -to be generated""", -) -op.add_option( - "-F", - "--file-list", - dest="file_list_file", - default=None, - help="""A file containing IPDL files to parse. This will be -merged with files provided on the commandline.""", -) - -options, cmdline_files = op.parse_args() -_verbosity = options.verbosity -syncMsgList = options.syncMsgList -msgMetadata = options.msgMetadata -headersdir = options.headersdir -cppdir = options.cppdir -includedirs = [os.path.abspath(incdir) for incdir in options.includedirs] - -files = [] - -if options.file_list_file is not None: - with open(options.file_list_file) as f: - files.extend(f.read().splitlines()) - -files.extend(cmdline_files) - -if not len(files): - op.error("No IPDL files specified") - -ipcmessagestartpath = os.path.join(headersdir, "IPCMessageStart.h") -ipc_msgtype_name_path = os.path.join(cppdir, "IPCMessageTypeName.cpp") - -log(2, 'Generated C++ headers will be generated relative to "%s"', headersdir) -log(2, 'Generated C++ sources will be generated in "%s"', cppdir) - -allmessages = {} -allsyncmessages = [] -allmessageprognames = [] -allprotocols = [] - - -def normalizedFilename(f): - if f == "-": - return "" - return f - - -log(2, "Reading sync message list") -parser = RawConfigParser() -parser.read_file(open(options.syncMsgList)) -syncMsgList = parser.sections() - -for section in syncMsgList: - if not parser.get(section, "description"): - print("Error: Sync message %s lacks a description" % section, file=sys.stderr) - sys.exit(1) - -# Read message metadata. Right now we only have 'segment_capacity' -# for the standard segment size used for serialization. -log(2, "Reading message metadata...") -msgMetadataConfig = RawConfigParser() -msgMetadataConfig.read_file(open(options.msgMetadata)) - -segmentCapacityDict = {} -for msgName in msgMetadataConfig.sections(): - if msgMetadataConfig.has_option(msgName, "segment_capacity"): - capacity = msgMetadataConfig.get(msgName, "segment_capacity") - segmentCapacityDict[msgName] = capacity - -# First pass: parse and type-check all protocols -for f in files: - log(2, os.path.basename(f)) - filename = normalizedFilename(f) - if f == "-": - fd = sys.stdin - else: - fd = open(f) - - specstring = fd.read() - fd.close() - - ast = ipdl.parse(specstring, filename, includedirs=includedirs) - if ast is None: - print("Specification could not be parsed.", file=sys.stderr) - sys.exit(1) +class WorkerPool: + per_process_context = None + + def __init__( + self, + manager, + files, + asts, + headersdir, + cppdir, + segmentCapacityDict, + allmessages, + allprotocols, + allmessageprognames, + allsyncmessages, + *, + processes=None + ): + if processes is None: + processes = os.cpu_count() or 1 + processes = min(processes, 8) + self.n = len(files) + self.asts = asts + self.pool = manager.Pool( + initializer=WorkerPool._init_worker, + initargs=( + files, + asts, + headersdir, + cppdir, + segmentCapacityDict, + allmessages, + allprotocols, + allmessageprognames, + allsyncmessages, + ), + processes=processes, + ) - log(2, "checking types") - if not ipdl.typecheck(ast): - print("Specification is not well typed.", file=sys.stderr) + def run(self): + self.pool.map(WorkerPool._run_worker, range(self.n)) + + @staticmethod + def _init_worker(*state): + WorkerPool.per_process_context = state + + @staticmethod + def _run_worker(index): + ( + files, + asts, + headersdir, + cppdir, + segmentCapacityDict, + allmessages, + allprotocols, + allmessageprognames, + allsyncmessages, + ) = WorkerPool.per_process_context + ast = asts[index] + ipdl.gencxx(files[index], ast, headersdir, cppdir, segmentCapacityDict) + + if ast.protocol: + allmessages[ast.protocol.name] = ipdl.genmsgenum(ast) + allprotocols.append(ast.protocol.name) + + # e.g. PContent::RequestMemoryReport (not prefixed or suffixed.) + for md in ast.protocol.messageDecls: + allmessageprognames.append("%s::%s" % (md.namespace, md.decl.progname)) + + if md.sendSemantics is SYNC: + allsyncmessages.append( + "%s__%s" % (ast.protocol.name, md.prettyMsgName()) + ) + + +def main(): + def log(minv, fmt, *args): + if _verbosity >= minv: + print(fmt % args) + + # process command line + + op = optparse.OptionParser(usage="ipdl.py [options] IPDLfiles...") + op.add_option( + "-I", + "--include", + dest="includedirs", + default=[], + action="append", + help="Additional directory to search for included protocol specifications", + ) + op.add_option( + "-s", + "--sync-msg-list", + dest="syncMsgList", + default="sync-messages.ini", + help="Config file listing allowed sync messages", + ) + op.add_option( + "-m", + "--msg-metadata", + dest="msgMetadata", + default="message-metadata.ini", + help="Predicted message sizes for reducing serialization malloc overhead.", + ) + op.add_option( + "-v", + "--verbose", + dest="verbosity", + default=1, + action="count", + help="Verbose logging (specify -vv or -vvv for very verbose logging)", + ) + op.add_option( + "-q", + "--quiet", + dest="verbosity", + action="store_const", + const=0, + help="Suppress logging output", + ) + op.add_option( + "-d", + "--outheaders-dir", + dest="headersdir", + default=".", + help="""Directory into which C++ headers will be generated. + A protocol Foo in the namespace bar will cause the headers + dir/bar/Foo.h, dir/bar/FooParent.h, and dir/bar/FooParent.h + to be generated""", + ) + op.add_option( + "-o", + "--outcpp-dir", + dest="cppdir", + default=".", + help="""Directory into which C++ sources will be generated + A protocol Foo in the namespace bar will cause the sources + cppdir/FooParent.cpp, cppdir/FooChild.cpp + to be generated""", + ) + op.add_option( + "-F", + "--file-list", + dest="file_list_file", + default=None, + help="""A file containing IPDL files to parse. This will be + merged with files provided on the commandline.""", + ) + + options, cmdline_files = op.parse_args() + _verbosity = options.verbosity + syncMsgList = options.syncMsgList + headersdir = options.headersdir + cppdir = options.cppdir + includedirs = [os.path.abspath(incdir) for incdir in options.includedirs] + + files = [] + + if options.file_list_file is not None: + with open(options.file_list_file) as f: + files.extend(f.read().splitlines()) + + files.extend(cmdline_files) + + if not len(files): + op.error("No IPDL files specified") + + ipcmessagestartpath = os.path.join(headersdir, "IPCMessageStart.h") + ipc_msgtype_name_path = os.path.join(cppdir, "IPCMessageTypeName.cpp") + + log(2, 'Generated C++ headers will be generated relative to "%s"', headersdir) + log(2, 'Generated C++ sources will be generated in "%s"', cppdir) + + def normalizedFilename(f): + if f == "-": + return "" + return f + + log(2, "Reading sync message list") + parser = RawConfigParser() + parser.read_file(open(options.syncMsgList)) + syncMsgList = parser.sections() + + for section in syncMsgList: + if not parser.get(section, "description"): + print( + "Error: Sync message %s lacks a description" % section, file=sys.stderr + ) + sys.exit(1) + + # Read message metadata. Right now we only have 'segment_capacity' + # for the standard segment size used for serialization. + log(2, "Reading message metadata...") + msgMetadataConfig = RawConfigParser() + msgMetadataConfig.read_file(open(options.msgMetadata)) + + manager = Manager() + segmentCapacityDict = manager.dict() + allmessages = manager.dict() + allsyncmessages = manager.list() + allmessageprognames = manager.list() + allprotocols = manager.list() + + for msgName in msgMetadataConfig.sections(): + if msgMetadataConfig.has_option(msgName, "segment_capacity"): + capacity = msgMetadataConfig.get(msgName, "segment_capacity") + segmentCapacityDict[msgName] = capacity + + # First pass: parse and type-check all protocols + asts = [] + for f in files: + log(2, os.path.basename(f)) + filename = normalizedFilename(f) + if f == "-": + fd = sys.stdin + else: + fd = open(f) + + specstring = fd.read() + fd.close() + + ast = ipdl.parse(specstring, filename, includedirs=includedirs) + if ast is None: + print("Specification could not be parsed.", file=sys.stderr) + sys.exit(1) + + log(2, "checking types") + if not ipdl.typecheck(ast): + print("Specification is not well typed.", file=sys.stderr) + sys.exit(1) + + if not ipdl.checkSyncMessage(ast, syncMsgList): + print( + "Error: New sync IPC messages must be reviewed by an IPC peer and recorded in %s" + % options.syncMsgList, + file=sys.stderr, + ) # NOQA: E501 + sys.exit(1) + + asts.append(ast) + + if not ipdl.checkFixedSyncMessages(parser): + # Errors have alraedy been printed to stderr, just exit sys.exit(1) - if not ipdl.checkSyncMessage(ast, syncMsgList): + # Second pass: generate code + pool = WorkerPool( + manager, + files, + asts, + headersdir, + cppdir, + segmentCapacityDict, + allmessages, + allprotocols, + allmessageprognames, + allsyncmessages, + ) + pool.run() + + allprotocols.sort() + allsyncmessages.sort() + + # Check if we have undefined message names in segmentCapacityDict. + # This is a fool-proof of the 'message-metadata.ini' file. + undefinedMessages = set(segmentCapacityDict.keys()) - set(allmessageprognames) + if len(undefinedMessages) > 0: print( - "Error: New sync IPC messages must be reviewed by an IPC peer and recorded in %s" - % options.syncMsgList, - file=sys.stderr, - ) # NOQA: E501 + "Error: Undefined message names in message-metadata.ini:", file=sys.stderr + ) + print(undefinedMessages, file=sys.stderr) sys.exit(1) -if not ipdl.checkFixedSyncMessages(parser): - # Errors have alraedy been printed to stderr, just exit - sys.exit(1) - -# Second pass: generate code -for f in files: - # Read from parser cache - filename = normalizedFilename(f) - ast = ipdl.parse(None, filename, includedirs=includedirs) - ipdl.gencxx(filename, ast, headersdir, cppdir, segmentCapacityDict) - - if ast.protocol: - allmessages[ast.protocol.name] = ipdl.genmsgenum(ast) - allprotocols.append(ast.protocol.name) - - # e.g. PContent::RequestMemoryReport (not prefixed or suffixed.) - for md in ast.protocol.messageDecls: - allmessageprognames.append("%s::%s" % (md.namespace, md.decl.progname)) - - if md.sendSemantics is SYNC: - allsyncmessages.append( - "%s__%s" % (ast.protocol.name, md.prettyMsgName()) - ) - -allprotocols.sort() + ipcmsgstart = StringIO() -# Check if we have undefined message names in segmentCapacityDict. -# This is a fool-proof of the 'message-metadata.ini' file. -undefinedMessages = set(segmentCapacityDict.keys()) - set(allmessageprognames) -if len(undefinedMessages) > 0: - print("Error: Undefined message names in message-metadata.ini:", file=sys.stderr) - print(undefinedMessages, file=sys.stderr) - sys.exit(1) - -ipcmsgstart = StringIO() - -print( - """ + print( + """ // CODE GENERATED by ipdl.py. Do not edit. #ifndef IPCMessageStart_h @@ -221,14 +298,14 @@ print( enum IPCMessageStart { """, - file=ipcmsgstart, -) + file=ipcmsgstart, + ) -for name in allprotocols: - print(" %sMsgStart," % name, file=ipcmsgstart) + for name in allprotocols: + print(" %sMsgStart," % name, file=ipcmsgstart) -print( - """ + print( + """ LastMsgIndex }; @@ -236,12 +313,12 @@ static_assert(LastMsgIndex <= 65536, "need to update IPC_MESSAGE_MACRO"); #endif // ifndef IPCMessageStart_h """, - file=ipcmsgstart, -) + file=ipcmsgstart, + ) -ipc_msgtype_name = StringIO() -print( - """ + ipc_msgtype_name = StringIO() + print( + """ // CODE GENERATED by ipdl.py. Do not edit. #include @@ -253,19 +330,19 @@ using std::uint32_t; namespace { enum IPCMessages { -""", - file=ipc_msgtype_name, -) - -for protocol in sorted(allmessages.keys()): - for msg, num in allmessages[protocol].idnums: - if num: - print(" %s = %s," % (msg, num), file=ipc_msgtype_name) - elif not msg.endswith("End"): - print(" %s__%s," % (protocol, msg), file=ipc_msgtype_name) - -print( - """ + """, + file=ipc_msgtype_name, + ) + + for protocol in sorted(allmessages.keys()): + for msg, num in allmessages[protocol].idnums: + if num: + print(" %s = %s," % (msg, num), file=ipc_msgtype_name) + elif not msg.endswith("End"): + print(" %s__%s," % (protocol, msg), file=ipc_msgtype_name) + + print( + """ }; } // anonymous namespace @@ -276,14 +353,14 @@ bool IPCMessageTypeIsSync(uint32_t aMessageType) { switch (aMessageType) { """, - file=ipc_msgtype_name, -) + file=ipc_msgtype_name, + ) -for msg in allsyncmessages: - print(" case %s:" % msg, file=ipc_msgtype_name) + for msg in allsyncmessages: + print(" case %s:" % msg, file=ipc_msgtype_name) -print( - """ return true; + print( + """ return true; default: return false; } @@ -292,24 +369,24 @@ print( const char* StringFromIPCMessageType(uint32_t aMessageType) { switch (aMessageType) { -""", - file=ipc_msgtype_name, -) - -for protocol in sorted(allmessages.keys()): - for msg, num in allmessages[protocol].idnums: - if num or msg.endswith("End"): - continue - print( - """ + """, + file=ipc_msgtype_name, + ) + + for protocol in sorted(allmessages.keys()): + for msg, num in allmessages[protocol].idnums: + if num or msg.endswith("End"): + continue + print( + """ case %s__%s: return "%s::%s";""" - % (protocol, msg, protocol, msg), - file=ipc_msgtype_name, - ) + % (protocol, msg, protocol, msg), + file=ipc_msgtype_name, + ) -print( - """ + print( + """ case DATA_PIPE_CLOSED_MESSAGE_TYPE: return "DATA_PIPE_CLOSED_MESSAGE"; case DATA_PIPE_BYTES_CONSUMED_MESSAGE_TYPE: @@ -353,15 +430,15 @@ namespace ipc { const char* ProtocolIdToName(IPCMessageStart aId) { switch (aId) { """, - file=ipc_msgtype_name, -) + file=ipc_msgtype_name, + ) -for name in allprotocols: - print(" case %sMsgStart:" % name, file=ipc_msgtype_name) - print(' return "%s";' % name, file=ipc_msgtype_name) + for name in allprotocols: + print(" case %sMsgStart:" % name, file=ipc_msgtype_name) + print(' return "%s";' % name, file=ipc_msgtype_name) -print( - """ + print( + """ default: return ""; } @@ -370,8 +447,12 @@ print( } // namespace ipc } // namespace mozilla """, - file=ipc_msgtype_name, -) + file=ipc_msgtype_name, + ) + + ipdl.writeifmodified(ipcmsgstart.getvalue(), ipcmessagestartpath) + ipdl.writeifmodified(ipc_msgtype_name.getvalue(), ipc_msgtype_name_path) + -ipdl.writeifmodified(ipcmsgstart.getvalue(), ipcmessagestartpath) -ipdl.writeifmodified(ipc_msgtype_name.getvalue(), ipc_msgtype_name_path) +if __name__ == "__main__": + main() diff --git a/ipc/ipdl/ipdl/__init__.py b/ipc/ipdl/ipdl/__init__.py index c042c16444..466a771c4e 100644 --- a/ipc/ipdl/ipdl/__init__.py +++ b/ipc/ipdl/ipdl/__init__.py @@ -85,7 +85,7 @@ def genmsgenum(ast): def writeifmodified(contents, file): contents = contents.encode("utf-8") dir = os.path.dirname(file) - os.path.exists(dir) or os.makedirs(dir) + os.makedirs(dir, exist_ok=True) oldcontents = None if os.path.exists(file): diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py index 291e1c698f..0e8b5e8dad 100644 --- a/ipc/ipdl/ipdl/lower.py +++ b/ipc/ipdl/ipdl/lower.py @@ -474,10 +474,6 @@ def errfnSentinel(rvalue=ExprLiteral.FALSE): return inner -def _destroyMethod(): - return ExprVar("ActorDestroy") - - def errfnUnreachable(msg): return [_logicError(msg)] @@ -3943,57 +3939,8 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): self.cls.addstmts([meth, refmeth, Whitespace.NL]) - # AllManagedActors(Array& inout) const - arrvar = ExprVar("arr__") - managedmeth = MethodDefn( - MethodDecl( - "AllManagedActors", - params=[ - Decl( - _cxxArrayType(_refptr(_cxxLifecycleProxyType()), ref=True), - arrvar.name, - ) - ], - methodspec=MethodSpec.OVERRIDE, - const=True, - ) - ) - - # Count the number of managed actors, and allocate space in the output array. - managedmeth.addcode( - """ - uint32_t total = 0; - """ - ) - for managed in ptype.manages: - managedmeth.addcode( - """ - total += ${container}.Count(); - """, - container=p.managedVar(managed, self.side), - ) - managedmeth.addcode( - """ - arr__.SetCapacity(total); - - """ - ) - - for managed in ptype.manages: - managedmeth.addcode( - """ - for (auto* key : ${container}) { - arr__.AppendElement(key->GetLifecycleProxy()); - } - - """, - container=p.managedVar(managed, self.side), - ) - - self.cls.addstmts([managedmeth, Whitespace.NL]) - # AllManagedActorsCount() const - managedmeth = MethodDefn( + managedcount = MethodDefn( MethodDecl( "AllManagedActorsCount", ret=Type.UINT32, @@ -4003,26 +3950,26 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): ) # Count the number of managed actors. - managedmeth.addcode( + managedcount.addcode( """ uint32_t total = 0; """ ) for managed in ptype.manages: - managedmeth.addcode( + managedcount.addcode( """ total += ${container}.Count(); """, container=p.managedVar(managed, self.side), ) - managedmeth.addcode( + managedcount.addcode( """ return total; """ ) - self.cls.addstmts([managedmeth, Whitespace.NL]) + self.cls.addstmts([managedcount, Whitespace.NL]) # OpenPEndpoint(...)/BindPEndpoint(...) for managed in ptype.manages: @@ -4188,66 +4135,50 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): ] ) - clearsubtreevar = ExprVar("ClearSubtree") + # DoomSubtree() + doomsubtree = MethodDefn( + MethodDecl("DoomSubtree", methodspec=MethodSpec.OVERRIDE) + ) - if ptype.isToplevel(): - # OnChannelClose() - onclose = MethodDefn( - MethodDecl("OnChannelClose", methodspec=MethodSpec.OVERRIDE) - ) - onclose.addcode( + for managed in ptype.manages: + doomsubtree.addcode( """ - DestroySubtree(NormalShutdown); - ClearSubtree(); - DeallocShmems(); - if (GetLifecycleProxy()) { - GetLifecycleProxy()->Release(); + for (auto* key : ${container}) { + key->DoomSubtree(); } - """ + """, + container=p.managedVar(managed, self.side), ) - self.cls.addstmts([onclose, Whitespace.NL]) - # OnChannelError() - onerror = MethodDefn( - MethodDecl("OnChannelError", methodspec=MethodSpec.OVERRIDE) - ) - onerror.addcode( - """ - DestroySubtree(AbnormalShutdown); - ClearSubtree(); - DeallocShmems(); - if (GetLifecycleProxy()) { - GetLifecycleProxy()->Release(); - } - """ - ) - self.cls.addstmts([onerror, Whitespace.NL]) + doomsubtree.addcode("SetDoomed();\n") - # private methods - self.cls.addstmt(Label.PRIVATE) + self.cls.addstmts([doomsubtree, Whitespace.NL]) + + # IProtocol* PeekManagedActor() override + peekmanagedactor = MethodDefn( + MethodDecl( + "PeekManagedActor", + methodspec=MethodSpec.OVERRIDE, + ret=Type("IProtocol", ptr=True), + ) + ) - # ClearSubtree() - clearsubtree = MethodDefn(MethodDecl(clearsubtreevar.name)) for managed in ptype.manages: - clearsubtree.addcode( + peekmanagedactor.addcode( """ - for (auto* key : ${container}) { - key->ClearSubtree(); - } - for (auto* key : ${container}) { - // Recursively releasing ${container} kids. - auto* proxy = key->GetLifecycleProxy(); - NS_IF_RELEASE(proxy); + if (IProtocol* actor = ${container}.Peek()) { + return actor; } - ${container}.Clear(); - """, container=p.managedVar(managed, self.side), ) - # don't release our own IPC reference: either the manager will do it, - # or we're toplevel - self.cls.addstmts([clearsubtree, Whitespace.NL]) + peekmanagedactor.addcode("return nullptr;\n") + + self.cls.addstmts([peekmanagedactor, Whitespace.NL]) + + # private methods + self.cls.addstmt(Label.PRIVATE) if not ptype.isToplevel(): self.cls.addstmts( @@ -4382,16 +4313,9 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): ) case = ExprCode( """ - { - ${manageecxxtype} actor = static_cast<${manageecxxtype}>(aListener); - - const bool removed = ${container}.EnsureRemoved(actor); - MOZ_RELEASE_ASSERT(removed, "actor not managed by this!"); + ${container}.EnsureRemoved(static_cast<${manageecxxtype}>(aListener)); + return; - auto* proxy = actor->GetLifecycleProxy(); - NS_IF_RELEASE(proxy); - return; - } """, manageecxxtype=manageecxxtype, container=p.managedVar(manageeipdltype, self.side), @@ -4708,14 +4632,18 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): return [ StmtCode( """ - if (!${actor}) { - NS_WARNING("Cannot bind null ${actorname} actor"); - return ${errfn}; - } + if (!${actor}) { + NS_WARNING("Cannot bind null ${actorname} actor"); + return ${errfn}; + } - ${actor}->SetManagerAndRegister($,{setManagerArgs}); - ${container}.Insert(${actor}); - """, + if (${actor}->SetManagerAndRegister($,{setManagerArgs})) { + ${container}.Insert(${actor}); + } else { + NS_WARNING("Failed to bind ${actorname} actor"); + return ${errfn}; + } + """, actor=actordecl.var(), actorname=actorproto.name() + self.side.capitalize(), errfn=errfn, @@ -4766,22 +4694,13 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): return method, (lbl, case) def destroyActor(self, md, actorexpr, why=_DestroyReason.Deletion): - if md and md.decl.type.isCtor(): - destroyedType = md.decl.type.constructedType() - else: - destroyedType = self.protocol.decl.type - return [ StmtCode( """ - IProtocol* mgr = ${actor}->Manager(); - ${actor}->DestroySubtree(${why}); - ${actor}->ClearSubtree(); - mgr->RemoveManagee(${protoId}, ${actor}); + ${actor}->ActorDisconnected(${why}); """, actor=actorexpr, why=why, - protoId=_protocolId(destroyedType), ) ] diff --git a/ipc/ipdl/test/gtest/PTestDestroyNested.ipdl b/ipc/ipdl/test/gtest/PTestDestroyNested.ipdl new file mode 100644 index 0000000000..1d58f1cf0c --- /dev/null +++ b/ipc/ipdl/test/gtest/PTestDestroyNested.ipdl @@ -0,0 +1,18 @@ +/* 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 protocol PTestDestroyNestedSub; + +namespace mozilla { +namespace _ipdltest { + +[ChildProc=any, ChildImpl=virtual, ParentImpl=virtual] +async protocol PTestDestroyNested { + manages PTestDestroyNestedSub; +child: + async PTestDestroyNestedSub(); +}; + +} // namespace _ipdltest +} // namespace mozilla diff --git a/ipc/ipdl/test/gtest/PTestDestroyNestedSub.ipdl b/ipc/ipdl/test/gtest/PTestDestroyNestedSub.ipdl new file mode 100644 index 0000000000..033c49261d --- /dev/null +++ b/ipc/ipdl/test/gtest/PTestDestroyNestedSub.ipdl @@ -0,0 +1,19 @@ +/* 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 protocol PTestDestroyNested; + +namespace mozilla { +namespace _ipdltest { + +[ChildProc=any, ChildImpl=virtual, ParentImpl=virtual] +async protocol PTestDestroyNestedSub { + manager PTestDestroyNested; +child: + + async __delete__(); +}; + +} // namespace _ipdltest +} // namespace mozilla diff --git a/ipc/ipdl/test/gtest/TestDestroyNested.cpp b/ipc/ipdl/test/gtest/TestDestroyNested.cpp new file mode 100644 index 0000000000..a348357b51 --- /dev/null +++ b/ipc/ipdl/test/gtest/TestDestroyNested.cpp @@ -0,0 +1,123 @@ +/* -*- 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/. */ + +/* + * Test an actor being destroyed during another ActorDestroy. + */ + +#include "gtest/gtest.h" + +#include "mozilla/_ipdltest/IPDLUnitTest.h" +#include "mozilla/_ipdltest/PTestDestroyNestedChild.h" +#include "mozilla/_ipdltest/PTestDestroyNestedParent.h" +#include "mozilla/_ipdltest/PTestDestroyNestedSubChild.h" +#include "mozilla/_ipdltest/PTestDestroyNestedSubParent.h" + +#include "mozilla/SpinEventLoopUntil.h" + +using namespace mozilla::ipc; + +namespace mozilla::_ipdltest { + +class TestDestroyNestedSubParent : public PTestDestroyNestedSubParent { + NS_INLINE_DECL_REFCOUNTING(TestDestroyNestedSubParent, override) + + void ActorDestroy(ActorDestroyReason aReason) override; + + bool mActorDestroyCalled = false; + bool mCloseManager = false; + + nsrefcnt GetRefCnt() const { return mRefCnt; } + + private: + ~TestDestroyNestedSubParent() = default; +}; + +class TestDestroyNestedSubChild : public PTestDestroyNestedSubChild { + NS_INLINE_DECL_REFCOUNTING(TestDestroyNestedSubChild, override) + private: + ~TestDestroyNestedSubChild() = default; +}; + +class TestDestroyNestedParent : public PTestDestroyNestedParent { + NS_INLINE_DECL_REFCOUNTING(TestDestroyNestedParent, override) + + void ActorDestroy(ActorDestroyReason aReason) override; + + bool mActorDestroyCalled = false; + + private: + ~TestDestroyNestedParent() = default; +}; + +class TestDestroyNestedChild : public PTestDestroyNestedChild { + NS_INLINE_DECL_REFCOUNTING(TestDestroyNestedChild, override) + + already_AddRefed AllocPTestDestroyNestedSubChild() + override { + return MakeAndAddRef(); + } + + private: + ~TestDestroyNestedChild() = default; +}; + +void TestDestroyNestedSubParent::ActorDestroy(ActorDestroyReason aReason) { + // Destroy our manager from within ActorDestroy() and assert that we don't + // re-enter. + EXPECT_FALSE(mActorDestroyCalled) << "re-entered ActorDestroy()"; + mActorDestroyCalled = true; + + if (mCloseManager) { + EXPECT_FALSE( + static_cast(Manager())->mActorDestroyCalled) + << "manager already destroyed"; + Manager()->Close(); + EXPECT_TRUE( + static_cast(Manager())->mActorDestroyCalled) + << "manager successfully destroyed"; + } + + // Make sure we also process any pending events, because we might be spinning + // a nested event loop within `ActorDestroy`. + NS_ProcessPendingEvents(nullptr); +} + +void TestDestroyNestedParent::ActorDestroy(ActorDestroyReason aReason) { + // Destroy our manager from within ActorDestroy() and assert that we don't + // re-enter. + EXPECT_FALSE(mActorDestroyCalled) << "re-entered ActorDestroy()"; + mActorDestroyCalled = true; +} + +IPDL_TEST(TestDestroyNested) { + auto p = MakeRefPtr(); + p->mCloseManager = true; + auto* rv1 = mActor->SendPTestDestroyNestedSubConstructor(p); + ASSERT_EQ(p, rv1) << "can't allocate Sub"; + + bool rv2 = PTestDestroyNestedSubParent::Send__delete__(p); + ASSERT_TRUE(rv2) + << "Send__delete__ failed"; + + ASSERT_TRUE(mActor->mActorDestroyCalled) + << "Parent not destroyed"; + ASSERT_TRUE(p->mActorDestroyCalled) + << "Sub not destroyed"; + + ASSERT_EQ(p->GetRefCnt(), 1u) << "Outstanding references to Sub remain"; + + // Try to allocate a new actor under the already-destroyed manager, and ensure + // that it is destroyed as expected. + auto p2 = MakeRefPtr(); + auto* rv3 = mActor->SendPTestDestroyNestedSubConstructor(p2); + ASSERT_EQ(rv3, nullptr) << "construction succeeded unexpectedly"; + + ASSERT_TRUE(p2->mActorDestroyCalled) + << "Sub not destroyed"; +} + +} // namespace mozilla::_ipdltest diff --git a/ipc/ipdl/test/gtest/moz.build b/ipc/ipdl/test/gtest/moz.build index 86fa2eca9e..5b60532026 100644 --- a/ipc/ipdl/test/gtest/moz.build +++ b/ipc/ipdl/test/gtest/moz.build @@ -22,6 +22,7 @@ SOURCES += [ "TestCrossProcessSemaphore.cpp", "TestDataStructures.cpp", "TestDescendant.cpp", + "TestDestroyNested.cpp", "TestEndpointOpens.cpp", "TestHangs.cpp", "TestInduceConnectionError.cpp", @@ -49,6 +50,8 @@ IPDL_SOURCES += [ "PTestDescendant.ipdl", "PTestDescendantSub.ipdl", "PTestDescendantSubsub.ipdl", + "PTestDestroyNested.ipdl", + "PTestDestroyNestedSub.ipdl", "PTestEndpointOpens.ipdl", "PTestEndpointOpensOpened.ipdl", "PTestHangs.ipdl", -- cgit v1.2.3