From 4e0fc495d66f9ab240fabcf493450bb36c433a5d Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Thu, 26 Apr 2018 17:14:22 -0700 Subject: [PATCH] Bug 1364624 - Part 1: Manually manage locks in MessageChannel::Close. r=froydnj This switches over to manually managing the locking in MessageChannel::Close in order to avoid a deadlock on msvc opt builds. It has the added benefit of avoid a superfluous lock/unlock pair. --HG-- extra : rebase_source : f3b0ee5499bd75bc75b3d1fe44c0c7efd3063aee --- ipc/glue/MessageChannel.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp index 59a229e4670d..11f263597fa5 100644 --- a/ipc/glue/MessageChannel.cpp +++ b/ipc/glue/MessageChannel.cpp @@ -13,6 +13,7 @@ #include "mozilla/ipc/ProtocolUtils.h" #include "mozilla/Logging.h" #include "mozilla/Move.h" +#include "mozilla/ScopeExit.h" #include "mozilla/Sprintf.h" #include "mozilla/Telemetry.h" #include "mozilla/TimeStamp.h" @@ -2696,7 +2697,16 @@ MessageChannel::Close() AssertWorkerThread(); { - MonitorAutoLock lock(*mMonitor); + // 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(); + + // Instead just use a ScopeExit to manage the unlock. + RefPtr monitor(mMonitor); + auto exit = MakeScopeExit([m = Move(monitor)] () { + m->Unlock(); + }); if (ChannelError == mChannelState || ChannelTimeout == mChannelState) { // See bug 538586: if the listener gets deleted while the @@ -2705,7 +2715,8 @@ MessageChannel::Close() // also be deleted and the listener will never be notified // of the channel error. if (mListener) { - MonitorAutoUnlock unlock(*mMonitor); + exit.release(); // Explicitly unlocking, clear scope exit. + mMonitor->Unlock(); NotifyMaybeChannelError(); } return;