Commit e2491522 authored by Nika Layzell's avatar Nika Layzell Committed by Richard Pospesel
Browse files

Bug 1789440 - Track reply message IDs for MessageChannel async replies,...

Bug 1789440 - Track reply message IDs for MessageChannel async replies, r=ipc-reviewers,mccr8 a=RyanVM

Differential Revision: https://phabricator.services.mozilla.com/D156569
parent 1b2bdfec
Loading
Loading
Loading
Loading
+4 −3
Original line number Diff line number Diff line
@@ -940,9 +940,10 @@ void MessageChannel::StopPostponingSends() {
}

UniquePtr<MessageChannel::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()) {
    UniquePtr<MessageChannel::UntypedCallbackHolder> ret =
        std::move(iter->second);
    mPendingResponses.erase(iter);
@@ -952,7 +953,7 @@ UniquePtr<MessageChannel::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) {
+20 −18
Original line number Diff line number Diff line
@@ -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.
+4 −2
Original line number Diff line number Diff line
@@ -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;
}
+5 −3
Original line number Diff line number Diff line
@@ -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) {
    UniquePtr<IPC::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);
+8 −2
Original line number Diff line number Diff line
@@ -4803,7 +4803,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor):
            $*{prologue}

            UniquePtr<MessageChannel::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),
                        ],
                    )
                )
            )