Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-03-02 22:37:50 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1719577 - Part 6: More consistently use the monitor during MessageChannel shutdown, r=handyman

Browse Source 
This should make the logic around clearing a MessageChannel more obviously
correct by holding the mutex when accessing fields which are traditionally
guarded by the mutex. These lock calls shouldn't introduce performance issues
as the lock should be uncontended.

Differential Revision: https://phabricator.services.mozilla.com/D119354
This commit is contained in:
Nika Layzell 2021-07-23 19:14:58 +00:00
parent 93f8303221
commit 639ed70cb4
3 changed files with 129 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

226
ipc/glue/MessageChannel.cpp
View File

@ -127,7 +127,7 @@ using mozilla::dom::AutoNoJSAPI;
if (!(_cond)) DebugAbort(__FILE__, __LINE__, #_cond, ##__VA_ARGS__); \
} while (0)
static MessageChannel* gParentProcessBlocker;
static MessageChannel* gParentProcessBlocker = nullptr;
namespace mozilla {
namespace ipc {
@ -584,6 +584,7 @@ MessageChannel::MessageChannel(const char* aName, IToplevelProtocol* aListener)
MessageChannel::~MessageChannel() {
MOZ_COUNT_DTOR(ipc::MessageChannel);
MonitorAutoLock lock(*mMonitor);
IPC_ASSERT(mCxxStackFrames.empty(), "mismatched CxxStackFrame ctor/dtors");
#ifdef OS_WIN
if (mEvent) {
@ -600,11 +601,54 @@ MessageChannel::~MessageChannel() {
<< "MessageChannel destructor ran without an mEvent Handle";
}
#endif
Clear();
// Make sure that the MessageChannel was closed (and therefore cleared) before
// it was destroyed. We can't properly close the channel at this point, as it
// would be unsafe to invoke our listener's callbacks, and we may be being
// destroyed on a thread other than `mWorkerThread`.
if (!Unsound_IsClosed()) {
CrashReporter::AnnotateCrashReport(
CrashReporter::Annotation::IPCFatalErrorProtocol,
nsDependentCString(mName));
switch (mChannelState) {
case ChannelConnected:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelConnected).");
break;
case ChannelTimeout:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelTimeout).");
break;
case ChannelClosing:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelClosing).");
break;
case ChannelError:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelError).");
break;
default:
MOZ_CRASH("MessageChannel destroyed without being closed.");
}
}
// Double-check other properties for thoroughness.
MOZ_RELEASE_ASSERT(!mLink);
MOZ_RELEASE_ASSERT(mPendingResponses.empty());
MOZ_RELEASE_ASSERT(!mChannelErrorTask);
MOZ_RELEASE_ASSERT(mPending.isEmpty());
MOZ_RELEASE_ASSERT(mOutOfTurnReplies.empty());
MOZ_RELEASE_ASSERT(mDeferred.empty());
}
#ifdef DEBUG
void MessageChannel::AssertMaybeDeferredCountCorrect() {
mMonitor->AssertCurrentThreadOwns();
size_t count = 0;
for (MessageTask* task : mPending) {
if (!IsAlwaysDeferred(task->Msg())) {
@ -678,65 +722,34 @@ bool MessageChannel::CanSend() const {
}
void MessageChannel::Clear() {
AssertWorkerThread();
mMonitor->AssertCurrentThreadOwns();
MOZ_DIAGNOSTIC_ASSERT(Unsound_IsClosed(),
"MessageChannel cleared too early?");
// Don't clear mWorkerThread; we use it in AssertWorkerThread().
//
// Also don't clear mListener. If we clear it, then sending a message
// through this channel after it's Clear()'ed can cause this process to
// crash.
//
// In practice, mListener owns the channel, so the channel gets deleted
// before mListener. But just to be safe, mListener is a weak pointer.
#if !defined(ANDROID)
if (!Unsound_IsClosed()) {
CrashReporter::AnnotateCrashReport(
CrashReporter::Annotation::IPCFatalErrorProtocol,
nsDependentCString(mName));
switch (mChannelState) {
case ChannelConnected:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelConnected).");
break;
case ChannelTimeout:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelTimeout).");
break;
case ChannelClosing:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelClosing).");
break;
case ChannelError:
MOZ_CRASH(
"MessageChannel destroyed without being closed "
"(mChannelState == ChannelError).");
break;
default:
MOZ_CRASH("MessageChannel destroyed without being closed.");
}
}
#endif
if (gParentProcessBlocker == this) {
if (NS_IsMainThread() && gParentProcessBlocker == this) {
gParentProcessBlocker = nullptr;
}
gUnresolvedResponses -= mPendingResponses.size();
for (auto& pair : mPendingResponses) {
pair.second.get()->Reject(ResponseRejectReason::ChannelClosed);
{
CallbackMap map = std::move(mPendingResponses);
MonitorAutoUnlock unlock(*mMonitor);
for (auto& pair : map) {
pair.second->Reject(ResponseRejectReason::ChannelClosed);
}
}
mPendingResponses.clear();
if (mLink != nullptr && mIsCrossProcess) {
ChannelCountReporter::Decrement(mName);
}
SetIsCrossProcess(false);
if (mLink) {
mLink->PrepareToDestroy();
mLink = nullptr;
}
mLink = nullptr;
if (mChannelErrorTask) {
mChannelErrorTask->Cancel();
@ -2407,15 +2420,17 @@ void MessageChannel::OnChannelErrorFromLink() {
PostErrorNotifyTask();
}
void MessageChannel::NotifyMaybeChannelError() {
mMonitor->AssertNotCurrentThreadOwns();
void MessageChannel::NotifyMaybeChannelError(Maybe<MonitorAutoLock>& aLock) {
AssertWorkerThread();
mMonitor->AssertCurrentThreadOwns();
MOZ_RELEASE_ASSERT(aLock.isSome());
// TODO sort out Close() on this side racing with Close() on the other side
if (ChannelClosing == mChannelState) {
// the channel closed, but we received a "Goodbye" message warning us
// about it. no worries
mChannelState = ChannelClosed;
NotifyChannelClosed();
NotifyChannelClosed(aLock);
return;
}
@ -2431,9 +2446,11 @@ void MessageChannel::NotifyMaybeChannelError() {
}
mNotifiedChannelDone = true;
// After this, the channel may be deleted. Based on the premise that
// mListener owns this channel, any calls back to this class that may
// work with mListener should still work on living objects.
// Let our listener know that the channel errored. This may cause the
// channel to be deleted. Release our caller's `MonitorAutoLock` before
// invoking the listener, as this may call back into MessageChannel, and/or
// cause the channel to be destroyed.
aLock.reset();
mListener->OnChannelError();
}
@ -2441,43 +2458,40 @@ void MessageChannel::OnNotifyMaybeChannelError() {
AssertWorkerThread();
mMonitor->AssertNotCurrentThreadOwns();
// This lock guard may be reset by `NotifyMaybeChannelError` before invoking
// listener callbacks which may destroy this `MessageChannel`.
//
// Acquiring the lock here also allows us to ensure that
// `OnChannelErrorFromLink` has finished running before this task is allowed
// to continue.
Maybe<MonitorAutoLock> lock(std::in_place, *mMonitor);
mChannelErrorTask = nullptr;
// OnChannelError holds mMonitor when it posts this task and this
// task cannot be allowed to run until OnChannelError has
// exited. We enforce that order by grabbing the mutex here which
// should only continue once OnChannelError has completed.
{
MonitorAutoLock lock(*mMonitor);
// nothing to do here
}
if (IsOnCxxStack()) {
mChannelErrorTask = NewNonOwningCancelableRunnableMethod(
"ipc::MessageChannel::OnNotifyMaybeChannelError", this,
&MessageChannel::OnNotifyMaybeChannelError);
RefPtr<Runnable> task = mChannelErrorTask;
// This used to post a 10ms delayed patch; however not all
// This used to post a 10ms delayed task; however not all
// nsISerialEventTarget implementations support delayed dispatch.
// The delay being completely arbitrary, we may not as well have any.
mWorkerThread->Dispatch(task.forget());
PostErrorNotifyTask();
return;
}
NotifyMaybeChannelError();
// This call may destroy `this`.
NotifyMaybeChannelError(lock);
}
void MessageChannel::PostErrorNotifyTask() {
mMonitor->AssertCurrentThreadOwns();
if (mChannelErrorTask) return;
if (mChannelErrorTask) {
return;
}
// This must be the last code that runs on this thread!
mChannelErrorTask = NewNonOwningCancelableRunnableMethod(
"ipc::MessageChannel::OnNotifyMaybeChannelError", this,
&MessageChannel::OnNotifyMaybeChannelError);
RefPtr<Runnable> task = mChannelErrorTask;
mWorkerThread->Dispatch(task.forget());
mWorkerThread->Dispatch(do_AddRef(mChannelErrorTask));
}
// Special async message.
@ -2535,53 +2549,47 @@ void MessageChannel::NotifyImpendingShutdown() {
void MessageChannel::Close() {
AssertWorkerThread();
mMonitor->AssertNotCurrentThreadOwns();
{
// We don't use MonitorAutoLock here as that causes some sort of
// deadlock in the error/timeout-with-a-listener state below when
// compiling an optimized msvc build.
mMonitor->Lock();
// This lock guard may be reset by `Notify{ChannelClosed,MaybeChannelError}`
// before invoking listener callbacks which may destroy this `MessageChannel`.
Maybe<MonitorAutoLock> lock(std::in_place, *mMonitor);
// Instead just use a ScopeExit to manage the unlock.
RefPtr<RefCountedMonitor> monitor(mMonitor);
auto exit = MakeScopeExit([m = std::move(monitor)]() { m->Unlock(); });
if (ChannelError == mChannelState || ChannelTimeout == mChannelState) {
switch (mChannelState) {
case ChannelError:
case ChannelTimeout:
// See bug 538586: if the listener gets deleted while the
// IO thread's NotifyChannelError event is still enqueued
// and subsequently deletes us, then the error event will
// also be deleted and the listener will never be notified
// of the channel error.
if (mListener) {
exit.release(); // Explicitly unlocking, clear scope exit.
mMonitor->Unlock();
NotifyMaybeChannelError();
}
NotifyMaybeChannelError(lock);
return;
}
if (ChannelClosed == mChannelState) {
case ChannelClosed:
// Slightly unexpected but harmless; ignore. See bug 1554244.
return;
}
// Notify the other side that we're about to close our socket. If we've
// already received a Goodbye from the other side (and our state is
// ChannelClosing), there's no reason to send one.
if (ChannelConnected == mChannelState) {
mLink->SendMessage(MakeUnique<GoodbyeMessage>());
}
SynchronouslyClose();
default:
// Notify the other side that we're about to close our socket. If we've
// already received a Goodbye from the other side (and our state is
// ChannelClosing), there's no reason to send one.
if (ChannelConnected == mChannelState) {
mLink->SendMessage(MakeUnique<GoodbyeMessage>());
}
SynchronouslyClose();
NotifyChannelClosed(lock);
return;
}
NotifyChannelClosed();
}
void MessageChannel::NotifyChannelClosed() {
mMonitor->AssertNotCurrentThreadOwns();
void MessageChannel::NotifyChannelClosed(Maybe<MonitorAutoLock>& aLock) {
AssertWorkerThread();
mMonitor->AssertCurrentThreadOwns();
MOZ_RELEASE_ASSERT(aLock.isSome());
if (ChannelClosed != mChannelState)
if (ChannelClosed != mChannelState) {
MOZ_CRASH("channel should have been closed!");
}
Clear();
@ -2592,9 +2600,11 @@ void MessageChannel::NotifyChannelClosed() {
}
mNotifiedChannelDone = true;
// OK, the IO thread just closed the channel normally. Let the
// listener know about it. After this point the channel may be
// deleted.
// Let our listener know that the channel was closed. This may cause the
// channel to be deleted. Release our caller's `MonitorAutoLock` before
// invoking the listener, as this may call back into MessageChannel, and/or
// cause the channel to be destroyed.
aLock.reset();
mListener->OnChannelClose();
}
@ -2687,6 +2697,8 @@ void MessageChannel::EndTimeout() {
}
void MessageChannel::RepostAllMessages() {
mMonitor->AssertCurrentThreadOwns();
bool needRepost = false;
for (MessageTask* task : mPending) {
if (!task->IsScheduled()) {
@ -2800,6 +2812,8 @@ void MessageChannel::CancelCurrentTransaction() {
}
void CancelCPOWs() {
MOZ_ASSERT(NS_IsMainThread());
if (gParentProcessBlocker) {
mozilla::Telemetry::Accumulate(mozilla::Telemetry::IPC_TRANSACTION_CANCEL,
true);

12
ipc/glue/MessageChannel.h
View File

@ -519,9 +519,15 @@ class MessageChannel : HasResultCodes {
void OnChannelErrorFromLink();
private:
// Run on the not current thread.
void NotifyChannelClosed();
void NotifyMaybeChannelError();
// Clear this channel, and notify the listener that the channel has either
// closed or errored.
//
// These methods must be called on the worker thread, passing in a
// `Maybe<MonitorAutoLock>`. This lock guard will be reset before the listener
// is called, allowing for the mutex to be unlocked before the MessageChannel
// is potentially destroyed.
void NotifyChannelClosed(Maybe<MonitorAutoLock>& aLock);
void NotifyMaybeChannelError(Maybe<MonitorAutoLock>& aLock);
private:
void AssertWorkerThread() const {

4
ipc/glue/MessageLink.h
View File

@ -49,10 +49,6 @@ class MessageLink {
explicit MessageLink(MessageChannel* aChan);
virtual ~MessageLink();
// This is called immediately before the MessageChannel destroys its
// MessageLink. See the implementation in ThreadLink for details.
virtual void PrepareToDestroy(){};
// n.b.: These methods all require that the channel monitor is
// held when they are invoked.
virtual void SendMessage(mozilla::UniquePtr<Message> msg) = 0;