Bug 1514417 - Add mutex to avoid VR shmem be deadlock when VRService and VRSubmitFrame threads are accessing it. r=kip
☠☠ backed out by 6b9d943ecc2a ☠ ☠
authorDaosheng Mu <daoshengmu@gmail.com>
Fri, 25 Jan 2019 01:04:09 +0000
changeset 515429 e0034670b26aaea119876ed739167136b75e1ec9
parent 515428 c8f0d19844f67cd9a2189f612e4f1a865d697b04
child 515430 9e7fe63b341a5a3a0479df93d6ebe2ded42f586d
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskip
bugs1514417
milestone66.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 1514417 - Add mutex to avoid VR shmem be deadlock when VRService and VRSubmitFrame threads are accessing it. r=kip MozReview-Commit-ID: 7uXdypiXTUH Differential Revision: https://phabricator.services.mozilla.com/D17581
gfx/vr/gfxVRExternal.cpp
gfx/vr/gfxVRExternal.h
gfx/vr/service/VRService.cpp
gfx/vr/service/VRService.h
--- a/gfx/vr/gfxVRExternal.cpp
+++ b/gfx/vr/gfxVRExternal.cpp
@@ -421,18 +421,18 @@ bool VRDisplayExternal::PullState() {
                             mDisplayInfo.mControllerState);
 }
 #endif
 
 VRSystemManagerExternal::VRSystemManagerExternal(
     VRExternalShmem* aAPIShmem /* = nullptr*/)
     : mExternalShmem(aAPIShmem)
 #if !defined(MOZ_WIDGET_ANDROID)
-      ,
-      mSameProcess(aAPIShmem != nullptr)
+    , mMutex("VRSystemManagerExternal::mMutex")
+    , mSameProcess(aAPIShmem != nullptr)
 #endif
 {
 #if defined(XP_MACOSX)
   mShmemFD = 0;
 #elif defined(XP_WIN)
   mShmemFile = NULL;
 #elif defined(MOZ_WIDGET_ANDROID)
   mExternalStructFailed = false;
@@ -839,15 +839,19 @@ void VRSystemManagerExternal::PushState(
       memcpy((void*)&(mExternalShmem->browserState), aBrowserState,
              sizeof(VRBrowserState));
       if (aNotifyCond) {
         pthread_cond_signal((pthread_cond_t*)&(mExternalShmem->browserCond));
       }
       pthread_mutex_unlock((pthread_mutex_t*)&(mExternalShmem->browserMutex));
     }
 #else
+    // We need this MutexAutoLock to avoid mAPIShmem happens deadlock issue
+    // when both of VRService and VRSubmitFrame threads are writing/reading
+    // it from the memory.
+    MutexAutoLock lock(mMutex);
     mExternalShmem->browserGenerationA++;
     memcpy((void*)&(mExternalShmem->browserState), (void*)aBrowserState,
            sizeof(VRBrowserState));
     mExternalShmem->browserGenerationB++;
 #endif  // defined(MOZ_WIDGET_ANDROID)
   }
 }
--- a/gfx/vr/gfxVRExternal.h
+++ b/gfx/vr/gfxVRExternal.h
@@ -129,16 +129,17 @@ class VRSystemManagerExternal : public V
 
   bool mExternalStructFailed;
   bool mEnumerationCompleted;
 #endif
   bool mDoShutdown;
 
   volatile VRExternalShmem* mExternalShmem;
 #if !defined(MOZ_WIDGET_ANDROID)
+  Mutex mMutex;
   bool mSameProcess;
 #endif
   TimeStamp mEarliestRestartTime;
 
   void OpenShmem();
   void CloseShmem();
   void CheckForShutdown();
 };
--- a/gfx/vr/service/VRService.cpp
+++ b/gfx/vr/service/VRService.cpp
@@ -63,16 +63,19 @@ VRService::VRService()
       mBrowserState{},
       mBrowserGeneration(0),
       mServiceThread(nullptr),
       mShutdownRequested(false),
       mAPIShmem(nullptr),
       mTargetShmemFile(0),
       mLastHapticState{},
       mFrameStartTime{},
+#if !defined(MOZ_WIDGET_ANDROID)
+      mMutex("VRService::mMutex"),
+#endif
       mVRProcessEnabled(gfxPrefs::VRProcessEnabled()) {
   // When we have the VR process, we map the memory
   // of mAPIShmem from GPU process.
   // If we don't have the VR process, we will instantiate
   // mAPIShmem in VRService.
   if (!mVRProcessEnabled) {
     mAPIShmem = new VRExternalShmem();
     memset(mAPIShmem, 0, sizeof(VRExternalShmem));
@@ -451,16 +454,20 @@ void VRService::PullState(mozilla::gfx::
 
 #if defined(MOZ_WIDGET_ANDROID)
   if (pthread_mutex_lock((pthread_mutex_t*)&(mExternalShmem->browserMutex)) ==
       0) {
     memcpy(&aState, &tmp.browserState, sizeof(VRBrowserState));
     pthread_mutex_unlock((pthread_mutex_t*)&(mExternalShmem->browserMutex));
   }
 #else
+  // We need this MutexAutoLock to avoid mAPIShmem happens deadlock issue
+  // when both of VRService and VRSubmitFrame threads are writing/reading
+  // it from the memory.
+  MutexAutoLock lock(mMutex);
   VRExternalShmem tmp;
   if (mAPIShmem->browserGenerationA != mBrowserGeneration) {
     memcpy(&tmp, mAPIShmem, sizeof(VRExternalShmem));
     if (tmp.browserGenerationA == tmp.browserGenerationB &&
         tmp.browserGenerationA != 0 && tmp.browserGenerationA != -1) {
       memcpy(&aState, &tmp.browserState, sizeof(VRBrowserState));
       mBrowserGeneration = tmp.browserGenerationA;
     }
--- a/gfx/vr/service/VRService.h
+++ b/gfx/vr/service/VRService.h
@@ -58,16 +58,19 @@ class VRService {
   UniquePtr<VRSession> mSession;
   base::Thread* mServiceThread;
   bool mShutdownRequested;
 
   VRExternalShmem* MOZ_OWNING_REF mAPIShmem;
   base::ProcessHandle mTargetShmemFile;
   VRHapticState mLastHapticState[kVRHapticsMaxCount];
   TimeStamp mFrameStartTime[kVRFrameTimingHistoryDepth];
+#if !defined(MOZ_WIDGET_ANDROID)
+  Mutex mMutex;
+#endif
   // We store the value of gfxPrefs::VRProcessEnabled() in mVRProcessEnabled.
   // This allows us to read the value in the VRService destructor, after
   // gfxPrefs has been shut down.  We should investigate why gfxPrefs
   // is shutting down earlier - See bug xxx
   bool mVRProcessEnabled;
 
   bool IsInServiceThread();
   void UpdateHaptics();