From 6c7d6d7d7e7fd2b013efa40063b35cb7ce61a763 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 22 Jan 2016 17:45:43 -0800 Subject: [PATCH] Bug 1240985 - Add mLastError to track sync Send errors better (r=dvander) --- ipc/glue/MessageChannel.cpp | 30 +++++++++++++++++++++++------- ipc/glue/MessageChannel.h | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index 6186b9b8d9b3..c90cfed9ced5 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -316,6 +316,7 @@ MessageChannel::MessageChannel(MessageListener *aListener) mTimeoutMs(kNoTimeout), mInTimeoutSecondHalf(false), mNextSeqno(0), + mLastSendError(SyncSendError::SendSuccess), mAwaitingSyncReply(false), mAwaitingSyncReplyPriority(0), mDispatchingSyncMessage(false), @@ -570,9 +571,10 @@ MessageChannel::Send(Message* aMsg) class CancelMessage : public IPC::Message { public: - CancelMessage() : + CancelMessage(int transaction) : IPC::Message(MSG_ROUTING_NONE, CANCEL_MESSAGE_TYPE, PRIORITY_NORMAL) { + set_transaction_id(transaction); } static bool Read(const Message* msg) { return true; @@ -908,6 +910,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) // message receives a reply, we'll be able to send more sync messages // again. IPC_LOG("Send() failed due to previous timeout"); + mLastSendError = SyncSendError::PreviousTimeout; return false; } @@ -918,6 +921,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) // Don't allow sending CPOWs while we're dispatching a sync message. // If you want to do that, use sendRpcMessage instead. IPC_LOG("Prio forbids send"); + mLastSendError = SyncSendError::SendingCPOWWhileDispatchingSync; return false; } @@ -929,6 +933,8 @@ MessageChannel::Send(Message* aMsg, Message* aReply) // sync messages it can send are high-priority. Mainly we want to ensure // here that we don't return false for non-CPOW messages. MOZ_ASSERT(msg->priority() == IPC::Message::PRIORITY_HIGH); + IPC_LOG("Sending while dispatching urgent message"); + mLastSendError = SyncSendError::SendingCPOWWhileDispatchingUrgent; return false; } @@ -938,10 +944,9 @@ MessageChannel::Send(Message* aMsg, Message* aReply) { MOZ_ASSERT(DispatchingSyncMessage() || DispatchingAsyncMessage()); IPC_LOG("Cancel from Send"); - CancelMessage *cancel = new CancelMessage(); - cancel->set_transaction_id(mCurrentTransaction); - mLink->SendMessage(cancel); + CancelMessage *cancel = new CancelMessage(mCurrentTransaction); CancelCurrentTransactionInternal(); + mLink->SendMessage(cancel); } IPC_ASSERT(msg->is_sync(), "can only Send() sync messages here"); @@ -960,6 +965,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) if (!Connected()) { ReportConnectionError("MessageChannel::SendAndWait", msg); + mLastSendError = SyncSendError::NotConnectedBeforeSend; return false; } @@ -991,6 +997,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) ProcessPendingRequests(transaction, prio); if (WasTransactionCanceled(transaction, prio)) { IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction); + mLastSendError = SyncSendError::CancelledAfterSend; return false; } @@ -998,6 +1005,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) if (mRecvdErrors) { IPC_LOG("Error: seqno=%d, xid=%d", seqno, transaction); mRecvdErrors--; + mLastSendError = SyncSendError::ReplyError; return false; } @@ -1013,11 +1021,13 @@ MessageChannel::Send(Message* aMsg, Message* aReply) if (!Connected()) { ReportConnectionError("MessageChannel::SendAndWait"); + mLastSendError = SyncSendError::DisconnectedDuringSend; return false; } if (WasTransactionCanceled(transaction, prio)) { IPC_LOG("Other side canceled seqno=%d, xid=%d", seqno, transaction); + mLastSendError = SyncSendError::CancelledAfterSend; return false; } @@ -1033,14 +1043,18 @@ MessageChannel::Send(Message* aMsg, Message* aReply) // there would be no way to unset it. if (mRecvdErrors) { mRecvdErrors--; + mLastSendError = SyncSendError::ReplyError; return false; } if (mRecvd) { break; } + IPC_LOG("Timing out Send: xid=%d", transaction); + mTimedOutMessageSeqno = seqno; mTimedOutMessagePriority = prio; + mLastSendError = SyncSendError::TimedOut; return false; } } @@ -1054,6 +1068,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) *aReply = Move(*mRecvd); mRecvd = nullptr; + mLastSendError = SyncSendError::SendSuccess; return true; } @@ -1828,6 +1843,8 @@ MessageChannel::OnChannelErrorFromLink() AssertLinkThread(); mMonitor->AssertCurrentThreadOwns(); + IPC_LOG("OnChannelErrorFromLink"); + if (InterruptStackDepth() > 0) NotifyWorkerThread(); @@ -2143,10 +2160,9 @@ MessageChannel::CancelCurrentTransaction() IPC_LOG("Cancel requested: current xid=%d", mCurrentTransaction); MOZ_ASSERT(DispatchingSyncMessage()); - CancelMessage *cancel = new CancelMessage(); - cancel->set_transaction_id(mCurrentTransaction); - mLink->SendMessage(cancel); + CancelMessage *cancel = new CancelMessage(mCurrentTransaction); CancelCurrentTransactionInternal(); + mLink->SendMessage(cancel); } } diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index 72c25e12eae0..d7add375c91b 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -44,6 +44,19 @@ class RefCountedMonitor : public Monitor ~RefCountedMonitor() {} }; +enum class SyncSendError { + SendSuccess, + PreviousTimeout, + SendingCPOWWhileDispatchingSync, + SendingCPOWWhileDispatchingUrgent, + NotConnectedBeforeSend, + DisconnectedDuringSend, + CancelledBeforeSend, + CancelledAfterSend, + TimedOut, + ReplyError, +}; + class MessageChannel : HasResultCodes { friend class ProcessLink; @@ -132,6 +145,13 @@ class MessageChannel : HasResultCodes bool CanSend() const; + // If sending a sync message returns an error, this function gives a more + // descriptive error message. + SyncSendError LastSendError() const { + AssertWorkerThread(); + return mLastSendError; + } + // Currently only for debugging purposes, doesn't aquire mMonitor. ChannelState GetChannelState__TotallyRacy() const { return mChannelState; @@ -521,6 +541,9 @@ class MessageChannel : HasResultCodes static bool sIsPumpingMessages; + // If ::Send returns false, this gives a more descriptive error. + SyncSendError mLastSendError; + template class AutoSetValue { public: