From aa1457713c5bdfaabac8a06d6a5e3f3094199cc4 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 21 Jun 2017 13:40:18 -0700 Subject: [PATCH] Allow IPDL message sends to be deferred and re-sent as needed. (bug 1369529 part 2, r=billm) --HG-- extra : rebase_source : 46d2af94da6b38e8c2fe70fd4566650d8cec8fe4 --- ipc/glue/MessageChannel.cpp | 52 +++++++++++++++++-- ipc/glue/MessageChannel.h | 26 ++++++++++ ipc/glue/MessageLink.cpp | 8 ++- ipc/ipdl/sync-messages.ini | 2 + ipc/ipdl/test/cxx/PTestPaintThread.ipdl | 4 +- .../test/cxx/TestOffMainThreadPainting.cpp | 11 ++-- ipc/ipdl/test/cxx/TestOffMainThreadPainting.h | 3 +- 7 files changed, 95 insertions(+), 11 deletions(-) diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index 3f874ef0e481..e7898987b400 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -537,7 +537,8 @@ MessageChannel::MessageChannel(const char* aName, mNotifiedChannelDone(false), mFlags(REQUIRE_DEFAULT), mPeerPidSet(false), - mPeerPid(-1) + mPeerPid(-1), + mIsPostponingSends(false) { MOZ_COUNT_CTOR(ipc::MessageChannel); @@ -901,10 +902,53 @@ MessageChannel::Send(Message* aMsg) ReportConnectionError("MessageChannel", msg); return false; } - mLink->SendMessage(msg.forget()); + + SendMessageToLink(msg.forget()); return true; } +void +MessageChannel::SendMessageToLink(Message* aMsg) +{ + if (mIsPostponingSends) { + UniquePtr msg(aMsg); + mPostponedSends.push_back(Move(msg)); + return; + } + mLink->SendMessage(aMsg); +} + +void +MessageChannel::BeginPostponingSends() +{ + AssertWorkerThread(); + mMonitor->AssertNotCurrentThreadOwns(); + + MonitorAutoLock lock(*mMonitor); + { + MOZ_ASSERT(!mIsPostponingSends); + mIsPostponingSends = true; + } +} + +void +MessageChannel::StopPostponingSends() +{ + // Note: this can be called from any thread. + MonitorAutoLock lock(*mMonitor); + + MOZ_ASSERT(mIsPostponingSends); + + for (UniquePtr& iter : mPostponedSends) { + mLink->SendMessage(iter.release()); + } + + // We unset this after SendMessage so we can make correct thread + // assertions in MessageLink. + mIsPostponingSends = false; + mPostponedSends.clear(); +} + already_AddRefed MessageChannel::PopPromise(const Message& aMsg) { @@ -1365,6 +1409,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) msg->nested_level() < AwaitingSyncReplyNestedLevel()) { MOZ_RELEASE_ASSERT(DispatchingSyncMessage() || DispatchingAsyncMessage()); + MOZ_RELEASE_ASSERT(!mIsPostponingSends); IPC_LOG("Cancel from Send"); CancelMessage *cancel = new CancelMessage(CurrentNestedInsideSyncTransaction()); CancelTransaction(CurrentNestedInsideSyncTransaction()); @@ -1413,7 +1458,7 @@ MessageChannel::Send(Message* aMsg, Message* aReply) // msg will be destroyed soon, but name() is not owned by msg. const char* msgName = msg->name(); - mLink->SendMessage(msg.forget()); + SendMessageToLink(msg.forget()); while (true) { MOZ_RELEASE_ASSERT(!transact.IsCanceled()); @@ -1544,6 +1589,7 @@ MessageChannel::Call(Message* aMsg, Message* aReply) IPC_ASSERT(!DispatchingSyncMessage(), "violation of sync handler invariant"); IPC_ASSERT(msg->is_interrupt(), "can only Call() Interrupt messages here"); + IPC_ASSERT(!mIsPostponingSends, "not postponing sends"); msg->set_seqno(NextSeqno()); msg->set_interrupt_remote_stack_depth_guess(mRemoteStackDepthGuess); diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h index 9b9e8fe937ae..e0a3bb080bf8 100644 --- a/ipc/glue/MessageChannel.h +++ b/ipc/glue/MessageChannel.h @@ -33,6 +33,7 @@ #include #include #include +#include namespace mozilla { namespace ipc { @@ -254,6 +255,22 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver bool IsInTransaction() const; void CancelCurrentTransaction(); + // Force all calls to Send to defer actually sending messages. This will + // cause sync messages to block until another thread calls + // StopPostponingSends. + // + // This must be called from the worker thread. + void BeginPostponingSends(); + + // Stop postponing sent messages, and immediately flush all postponed + // messages to the link. This may be called from any thread. + // + // Note that there are no ordering guarantees between two different + // MessageChannels. If channel B sends a message, then stops postponing + // channel A, messages from A may arrive before B. The easiest way to order + // this, if needed, is to make B send a sync message. + void StopPostponingSends(); + /** * This function is used by hang annotation code to determine which IPDL * actor is highest in the call stack at the time of the hang. It should @@ -491,6 +508,10 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver // depending on context. static bool IsAlwaysDeferred(const Message& aMsg); + // Helper for sending a message via the link. This should only be used for + // non-special messages that might have to be postponed. + void SendMessageToLink(Message* aMsg); + bool WasTransactionCanceled(int transaction); bool ShouldDeferMessage(const Message& aMsg); bool ShouldDeferInterruptMessage(const Message& aMsg, size_t aStackDepth); @@ -797,6 +818,11 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver RefPtr mOnChannelConnectedTask; bool mPeerPidSet; int32_t mPeerPid; + + // Channels can enter messages are not sent immediately; instead, they are + // held in a queue until another thread deems it is safe to send them. + bool mIsPostponingSends; + std::vector> mPostponedSends; }; void diff --git a/ipc/glue/MessageLink.cpp b/ipc/glue/MessageLink.cpp index 9bc1a1165e11..4cf412b1d251 100644 --- a/ipc/glue/MessageLink.cpp +++ b/ipc/glue/MessageLink.cpp @@ -149,7 +149,9 @@ ProcessLink::SendMessage(Message *msg) MOZ_CRASH("IPC message size is too large"); } - mChan->AssertWorkerThread(); + if (!mChan->mIsPostponingSends) { + mChan->AssertWorkerThread(); + } mChan->mMonitor->AssertCurrentThreadOwns(); mIOLoop->PostTask(NewNonOwningRunnableMethod(mTransport, &Transport::Send, msg)); @@ -212,7 +214,9 @@ ThreadLink::EchoMessage(Message *msg) void ThreadLink::SendMessage(Message *msg) { - mChan->AssertWorkerThread(); + if (!mChan->mIsPostponingSends) { + mChan->AssertWorkerThread(); + } mChan->mMonitor->AssertCurrentThreadOwns(); if (mTargetChan) diff --git a/ipc/ipdl/sync-messages.ini b/ipc/ipdl/sync-messages.ini index 03bb1c3fd8b1..4a2c06a159a5 100644 --- a/ipc/ipdl/sync-messages.ini +++ b/ipc/ipdl/sync-messages.ini @@ -231,6 +231,8 @@ description = description = [PTestLayoutThread::SyncMessage] description = +[PTestPaintThread::FinishedPaint] +description = # A11y code [PDocAccessible::State] diff --git a/ipc/ipdl/test/cxx/PTestPaintThread.ipdl b/ipc/ipdl/test/cxx/PTestPaintThread.ipdl index 57ba44bf25e6..0a37fda1be33 100644 --- a/ipc/ipdl/test/cxx/PTestPaintThread.ipdl +++ b/ipc/ipdl/test/cxx/PTestPaintThread.ipdl @@ -3,10 +3,10 @@ namespace mozilla { namespace _ipdltest { // This is supposed to be analagous to PPaintingBridge. -protocol PTestPaintThread +sync protocol PTestPaintThread { parent: - async FinishedPaint(uint64_t aTxnId); + sync FinishedPaint(uint64_t aTxnId); }; } // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp b/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp index 4ecd47ab92c9..a9fc6b99bc48 100644 --- a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp +++ b/ipc/ipdl/test/cxx/TestOffMainThreadPainting.cpp @@ -141,7 +141,7 @@ TestOffMainThreadPaintingChild::RecvStartTest(ipc::Endpoint task = NewRunnableMethod&&>( "TestPaintthreadChild::Bind", mPaintActor, &TestPaintThreadChild::Bind, Move(aEndpoint)); mPaintThread->message_loop()->PostTask(task.forget()); @@ -170,6 +170,8 @@ TestOffMainThreadPaintingChild::ProcessingError(Result aCode, const char* aReaso void TestOffMainThreadPaintingChild::IssueTransaction() { + GetIPCChannel()->BeginPostponingSends(); + uint64_t txnId = mNextTxnId++; // Start painting before we send the message. @@ -230,8 +232,9 @@ TestPaintThreadParent::DeallocPTestPaintThreadParent() * PTestPaintThreadChild * *************************/ -TestPaintThreadChild::TestPaintThreadChild() - : mCanSend(false) +TestPaintThreadChild::TestPaintThreadChild(MessageChannel* aMainChannel) + : mCanSend(false), + mMainChannel(aMainChannel) { } @@ -257,6 +260,8 @@ TestPaintThreadChild::BeginPaintingForTxn(uint64_t aTxnId) // Sleep for some time to simulate painting being slow. PR_Sleep(PR_MillisecondsToInterval(500)); SendFinishedPaint(aTxnId); + + mMainChannel->StopPostponingSends(); } void diff --git a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h b/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h index d39702659454..0a9b01dc3c90 100644 --- a/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h +++ b/ipc/ipdl/test/cxx/TestOffMainThreadPainting.h @@ -89,7 +89,7 @@ private: class TestPaintThreadChild final : public PTestPaintThreadChild { public: - TestPaintThreadChild(); + explicit TestPaintThreadChild(MessageChannel* aOtherChannel); NS_INLINE_DECL_THREADSAFE_REFCOUNTING(TestPaintThreadChild); @@ -103,6 +103,7 @@ private: ~TestPaintThreadChild() override; bool mCanSend; + MessageChannel* mMainChannel; }; } // namespace _ipdltest