This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch tor-browser-91.13.0esr-11.5-1 in repository tor-browser.
commit e2491522e54dd23286d872b3518578589b84ff51 Author: Nika Layzell nika@thelayzells.com AuthorDate: Wed Sep 7 23:38:45 2022 +0000
Bug 1789440 - Track reply message IDs for MessageChannel async replies, r=ipc-reviewers,mccr8 a=RyanVM
Differential Revision: https://phabricator.services.mozilla.com/D156569 --- ipc/glue/MessageChannel.cpp | 7 ++++--- ipc/glue/MessageChannel.h | 38 ++++++++++++++++++++------------------ ipc/glue/ProtocolUtils.cpp | 6 ++++-- ipc/glue/ProtocolUtils.h | 8 +++++--- ipc/ipdl/ipdl/lower.py | 10 ++++++++-- 5 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index ac1c4d9a7b36..bb10e7f4610e 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -940,9 +940,10 @@ void MessageChannel::StopPostponingSends() { }
UniquePtrMessageChannel::UntypedCallbackHolder MessageChannel::PopCallback( - const Message& aMsg) { + const Message& aMsg, int32_t aActorId) { auto iter = mPendingResponses.find(aMsg.seqno()); - if (iter != mPendingResponses.end()) { + if (iter != mPendingResponses.end() && iter->second->mActorId == aActorId && + iter->second->mReplyMsgId == aMsg.type()) { UniquePtrMessageChannel::UntypedCallbackHolder ret = std::move(iter->second); mPendingResponses.erase(iter); @@ -952,7 +953,7 @@ UniquePtrMessageChannel::UntypedCallbackHolder MessageChannel::PopCallback( return nullptr; }
-void MessageChannel::RejectPendingResponsesForActor(ActorIdType aActorId) { +void MessageChannel::RejectPendingResponsesForActor(int32_t aActorId) { auto itr = mPendingResponses.begin(); while (itr != mPendingResponses.end()) { if (itr->second.get()->mActorId != aActorId) { diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index bc65f0e29bce..2c7f33736950 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -112,29 +112,30 @@ class MessageChannel : HasResultCodes {
typedef mozilla::Monitor Monitor;
- // We could templatize the actor type but it would unnecessarily - // expand the code size. Using the actor address as the - // identifier is already good enough. - typedef void* ActorIdType; - public: + using Message = IPC::Message; + struct UntypedCallbackHolder { - UntypedCallbackHolder(ActorIdType aActorId, RejectCallback&& aReject) - : mActorId(aActorId), mReject(std::move(aReject)) {} + UntypedCallbackHolder(int32_t aActorId, Message::msgid_t aReplyMsgId, + RejectCallback&& aReject) + : mActorId(aActorId), + mReplyMsgId(aReplyMsgId), + mReject(std::move(aReject)) {}
virtual ~UntypedCallbackHolder() = default;
void Reject(ResponseRejectReason&& aReason) { mReject(std::move(aReason)); }
- ActorIdType mActorId; + int32_t mActorId; + Message::msgid_t mReplyMsgId; RejectCallback mReject; };
template <typename Value> struct CallbackHolder : public UntypedCallbackHolder { - CallbackHolder(ActorIdType aActorId, ResolveCallback<Value>&& aResolve, - RejectCallback&& aReject) - : UntypedCallbackHolder(aActorId, std::move(aReject)), + CallbackHolder(int32_t aActorId, Message::msgid_t aReplyMsgId, + ResolveCallback<Value>&& aResolve, RejectCallback&& aReject) + : UntypedCallbackHolder(aActorId, aReplyMsgId, std::move(aReject)), mResolve(std::move(aResolve)) {}
void Resolve(Value&& aReason) { mResolve(std::move(aReason)); } @@ -149,7 +150,6 @@ class MessageChannel : HasResultCodes { public: static const int32_t kNoTimeout;
- typedef IPC::Message Message; typedef IPC::MessageInfo MessageInfo; typedef mozilla::ipc::Transport Transport; using ScopedPort = mozilla::ipc::ScopedPort; @@ -232,8 +232,9 @@ class MessageChannel : HasResultCodes { // Asynchronously send a message to the other side of the channel // and wait for asynchronous reply. template <typename Value> - void Send(UniquePtr<Message> aMsg, ActorIdType aActorId, - ResolveCallback<Value>&& aResolve, RejectCallback&& aReject) { + void Send(UniquePtr<Message> aMsg, int32_t aActorId, + Message::msgid_t aReplyMsgId, ResolveCallback<Value>&& aResolve, + RejectCallback&& aReject) { int32_t seqno = NextSeqno(); aMsg->set_seqno(seqno); if (!Send(std::move(aMsg))) { @@ -242,8 +243,8 @@ class MessageChannel : HasResultCodes { }
UniquePtr<UntypedCallbackHolder> callback = - MakeUnique<CallbackHolder<Value>>(aActorId, std::move(aResolve), - std::move(aReject)); + MakeUnique<CallbackHolder<Value>>( + aActorId, aReplyMsgId, std::move(aResolve), std::move(aReject)); mPendingResponses.insert(std::make_pair(seqno, std::move(callback))); gUnresolvedResponses++; } @@ -263,11 +264,12 @@ class MessageChannel : HasResultCodes { bool CanSend() const;
// Remove and return a callback that needs reply - UniquePtr<UntypedCallbackHolder> PopCallback(const Message& aMsg); + UniquePtr<UntypedCallbackHolder> PopCallback(const Message& aMsg, + int32_t aActorId);
// Used to reject and remove pending responses owned by the given // actor when it's about to be destroyed. - void RejectPendingResponsesForActor(ActorIdType aActorId); + void RejectPendingResponsesForActor(int32_t aActorId);
// If sending a sync message returns an error, this function gives a more // descriptive error message. diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp index ea88763c9af2..4f46c1965be1 100644 --- a/ipc/glue/ProtocolUtils.cpp +++ b/ipc/glue/ProtocolUtils.cpp @@ -585,9 +585,11 @@ void IProtocol::DestroySubtree(ActorDestroyReason aWhy) { MOZ_ASSERT(CanRecv(), "destroying non-connected actor"); MOZ_ASSERT(mLifecycleProxy, "destroying zombie actor");
+ int32_t id = Id(); + // If we're a managed actor, unregister from our manager if (Manager()) { - Unregister(Id()); + Unregister(id); }
// Destroy subtree @@ -614,7 +616,7 @@ void IProtocol::DestroySubtree(ActorDestroyReason aWhy) { // The actor is being destroyed, reject any pending responses, invoke // `ActorDestroy` to destroy it, and then clear our status to // `LinkStatus::Destroyed`. - GetIPCChannel()->RejectPendingResponsesForActor(this); + GetIPCChannel()->RejectPendingResponsesForActor(id); ActorDestroy(aWhy); mLinkStatus = LinkStatus::Destroyed; } diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index 3d024686727a..a0b64b05ccfd 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -302,12 +302,14 @@ class IProtocol : public HasResultCodes { bool ChannelSend(IPC::Message* aMsg, IPC::Message* aReply); bool ChannelCall(IPC::Message* aMsg, IPC::Message* aReply); template <typename Value> - void ChannelSend(IPC::Message* aMsg, ResolveCallback<Value>&& aResolve, + void ChannelSend(IPC::Message* aMsg, + IPC::Message::msgid_t aReplyMsgId, + ResolveCallback<Value>&& aResolve, RejectCallback&& aReject) { UniquePtrIPC::Message msg(aMsg); if (CanSend()) { - GetIPCChannel()->Send(std::move(msg), this, std::move(aResolve), - std::move(aReject)); + GetIPCChannel()->Send(std::move(msg), Id(), aReplyMsgId, + std::move(aResolve), std::move(aReject)); } else { NS_WARNING("IPC message discarded: actor cannot send"); aReject(ResponseRejectReason::SendError); diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py index e40cbcf46353..fbd174032e46 100644 --- a/ipc/ipdl/ipdl/lower.py +++ b/ipc/ipdl/ipdl/lower.py @@ -4803,7 +4803,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): $*{prologue}
UniquePtrMessageChannel::UntypedCallbackHolder untypedCallback = - GetIPCChannel()->PopCallback(${msgvar}); + GetIPCChannel()->PopCallback(${msgvar}, Id());
typedef MessageChannel::CallbackHolder<${resolvetype}> CallbackHolder; auto* callback = static_cast<CallbackHolder*>(untypedCallback.get()); @@ -5342,7 +5342,13 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): stmts.append( StmtExpr( ExprCall( - send, args=[msgexpr, ExprMove(resolvefn), ExprMove(rejectfn)] + send, + args=[ + ExprMove(msgexpr), + ExprVar(md.pqReplyId()), + ExprMove(resolvefn), + ExprMove(rejectfn), + ], ) ) )