Bug 1441498 - Protect mCompositorVsyncDispatcher by lock. r=kats
authorSotaro Ikeda <sikeda@mozilla.com>
Wed, 07 Mar 2018 10:53:05 -0500
changeset 461979 76a8f710da41058f6825d45aa8759c954812b488
parent 461978 5138dcf1eddd750c06e05a784f170b771ba58f60
child 461980 3ac5bb463708fceec0e4d5812ee72f06fb40182c
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1441498
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1441498 - Protect mCompositorVsyncDispatcher by lock. r=kats mCompositorVsyncDispatcher is created and destroyed on main thread. But it is accessed from Compositor thread or VSyncBridge thread. To avoid race condition between main thread and Compositor thread/VSyncBridge thread, mCompositorVsyncDispatcher needs to be protected by lock.
widget/nsBaseWidget.cpp
widget/nsBaseWidget.h
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -258,16 +258,19 @@ nsBaseWidget::Shutdown()
 
 void nsBaseWidget::DestroyCompositor()
 {
   // We release this before releasing the compositor, since it may hold the
   // last reference to our ClientLayerManager. ClientLayerManager's dtor can
   // trigger a paint, creating a new compositor, and we don't want to re-use
   // the old vsync dispatcher.
   if (mCompositorVsyncDispatcher) {
+    MOZ_ASSERT(mCompositorVsyncDispatcherLock.get());
+
+    MutexAutoLock lock(*mCompositorVsyncDispatcherLock.get());
     mCompositorVsyncDispatcher->Shutdown();
     mCompositorVsyncDispatcher = nullptr;
   }
 
   // The compositor shutdown sequence looks like this:
   //  1. CompositorSession calls CompositorBridgeChild::Destroy.
   //  2. CompositorBridgeChild synchronously sends WillClose.
   //  3. CompositorBridgeParent releases some resources (such as the layer
@@ -1262,23 +1265,30 @@ nsBaseWidget::GetDocument() const
 }
 
 void nsBaseWidget::CreateCompositorVsyncDispatcher()
 {
   // Parent directly listens to the vsync source whereas
   // child process communicate via IPC
   // Should be called AFTER gfxPlatform is initialized
   if (XRE_IsParentProcess()) {
+    if (!mCompositorVsyncDispatcherLock) {
+      mCompositorVsyncDispatcherLock = MakeUnique<Mutex>("mCompositorVsyncDispatcherLock");
+    }
+    MutexAutoLock lock(*mCompositorVsyncDispatcherLock.get());
     mCompositorVsyncDispatcher = new CompositorVsyncDispatcher();
   }
 }
 
 already_AddRefed<CompositorVsyncDispatcher>
 nsBaseWidget::GetCompositorVsyncDispatcher()
 {
+  MOZ_ASSERT(mCompositorVsyncDispatcherLock.get());
+
+  MutexAutoLock lock(*mCompositorVsyncDispatcherLock.get());
   RefPtr<CompositorVsyncDispatcher> dispatcher = mCompositorVsyncDispatcher;
   return dispatcher.forget();
 }
 
 already_AddRefed<LayerManager>
 nsBaseWidget::CreateCompositorSession(int aWidth,
                                       int aHeight,
                                       CompositorOptions* aOptionsOut)
--- a/widget/nsBaseWidget.h
+++ b/widget/nsBaseWidget.h
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #ifndef nsBaseWidget_h__
 #define nsBaseWidget_h__
 
 #include "InputData.h"
 #include "mozilla/EventForwards.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/WidgetUtils.h"
 #include "mozilla/layers/APZCCallbackHelper.h"
 #include "mozilla/layers/CompositorOptions.h"
 #include "nsRect.h"
 #include "nsIWidget.h"
 #include "nsWidgetsCID.h"
@@ -674,17 +675,20 @@ protected:
   void FreeShutdownObserver();
 
   nsIWidgetListener* mWidgetListener;
   nsIWidgetListener* mAttachedWidgetListener;
   nsIWidgetListener* mPreviouslyAttachedWidgetListener;
   RefPtr<LayerManager> mLayerManager;
   RefPtr<CompositorSession> mCompositorSession;
   RefPtr<CompositorBridgeChild> mCompositorBridgeChild;
+
+  mozilla::UniquePtr<mozilla::Mutex> mCompositorVsyncDispatcherLock;
   RefPtr<mozilla::CompositorVsyncDispatcher> mCompositorVsyncDispatcher;
+
   RefPtr<IAPZCTreeManager> mAPZC;
   RefPtr<GeckoContentController> mRootContentController;
   RefPtr<APZEventState> mAPZEventState;
   SetAllowedTouchBehaviorCallback mSetAllowedTouchBehaviorCallback;
   RefPtr<WidgetShutdownObserver> mShutdownObserver;
   RefPtr<TextEventDispatcher> mTextEventDispatcher;
   nsCursor          mCursor;
   nsBorderStyle     mBorderStyle;