From 2649cb4bf48e4ab423a479774ae98389413fd992 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Thu, 21 Jan 2010 20:04:10 -0600 Subject: [PATCH] Bug 521929, part 2: Save racy RPC replies onto a special stack until they're the reply to the right out-call. r=bent --HG-- extra : transplant_source : %95R%85%B4%AD%0F%3D%9B%A5%18n%9B%94%BF%DA%9A%1BE%40%AC --- ipc/glue/RPCChannel.cpp | 33 +++++++++++++++++++++++++++++---- ipc/glue/RPCChannel.h | 7 +++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/ipc/glue/RPCChannel.cpp b/ipc/glue/RPCChannel.cpp index e6551a3f53b..2510a392e44 100644 --- a/ipc/glue/RPCChannel.cpp +++ b/ipc/glue/RPCChannel.cpp @@ -67,6 +67,7 @@ RPCChannel::RPCChannel(RPCListener* aListener, : SyncChannel(aListener), mPending(), mStack(), + mOutOfTurnReplies(), mDeferred(), mRemoteStackDepthGuess(0), mRacePolicy(aPolicy) @@ -112,7 +113,9 @@ RPCChannel::Call(Message* msg, Message* reply) // here we're waiting for something to happen. see long // comment about the queue in RPCChannel.h - while (Connected() && mPending.empty()) { + while (Connected() && mPending.empty() && + (mOutOfTurnReplies.empty() || + mOutOfTurnReplies.top().seqno() < mStack.top().seqno())) { WaitForNotify(); } @@ -121,8 +124,16 @@ RPCChannel::Call(Message* msg, Message* reply) return false; } - Message recvd = mPending.front(); - mPending.pop(); + Message recvd; + if (!mOutOfTurnReplies.empty() && + mOutOfTurnReplies.top().seqno() == mStack.top().seqno()) { + recvd = mOutOfTurnReplies.top(); + mOutOfTurnReplies.pop(); + } + else { + recvd = mPending.front(); + mPending.pop(); + } if (!recvd.is_sync() && !recvd.is_rpc()) { MutexAutoUnlock unlock(mMutex); @@ -145,6 +156,11 @@ RPCChannel::Call(Message* msg, Message* reply) const Message& outcall = mStack.top(); + if (recvd.seqno() < outcall.seqno()) { + mOutOfTurnReplies.push(recvd); + continue; + } + // FIXME/cjones: handle error RPC_ASSERT( recvd.is_reply_error() || @@ -161,7 +177,7 @@ RPCChannel::Call(Message* msg, Message* reply) *reply = recvd; } - if (0 == StackDepth()) + if (0 == StackDepth()) { // we may have received new messages while waiting for // our reply. because we were awaiting a reply, // StackDepth > 0, and the IO thread didn't enqueue @@ -169,6 +185,13 @@ RPCChannel::Call(Message* msg, Message* reply) // "losing" the new messages, we do that now. EnqueuePendingMessages(); + + RPC_ASSERT( + mOutOfTurnReplies.empty(), + "still have pending replies with no pending out-calls", + "rpc", true); + } + // finished with this RPC stack frame return !isError; } @@ -385,6 +408,8 @@ RPCChannel::DebugAbort(const char* file, int line, const char* cond, mRemoteStackDepthGuess); fprintf(stderr, " deferred stack size: %lu\n", mDeferred.size()); + fprintf(stderr, " out-of-turn RPC replies stack size: %lu\n", + mOutOfTurnReplies.size()); fprintf(stderr, " Pending queue size: %lu, front to back:\n", mPending.size()); while (!mPending.empty()) { diff --git a/ipc/glue/RPCChannel.h b/ipc/glue/RPCChannel.h index f41e2f41c5d..b7832cd0784 100644 --- a/ipc/glue/RPCChannel.h +++ b/ipc/glue/RPCChannel.h @@ -154,6 +154,13 @@ private: // std::stack mStack; + // + // Stack of replies received "out of turn", because of RPC + // in-calls racing with replies to outstanding in-calls. See + // https://bugzilla.mozilla.org/show_bug.cgi?id=521929. + // + std::stack mOutOfTurnReplies; + // // Stack of RPC in-calls that were deferred because of race // conditions.