Bug 1523926 - Fix open VR shmem mutex failed issue when without VR process. r=kip, a=RyanVM DEVEDITION_66_0b6_BUILD1 DEVEDITION_66_0b6_RELEASE FENNEC_66_0b6_BUILD1 FENNEC_66_0b6_RELEASE FIREFOX_66_0b6_BUILD1 FIREFOX_66_0b6_RELEASE
authorDaosheng Mu <daoshengmu@gmail.com>
Mon, 04 Feb 2019 15:19:33 +0000
changeset 515818 a9f6e3381fb3e64a6b872efa52a46afe4dd588e8
parent 515817 bf7ac7d809cf4edbb96f56e75ace2fed59a3e41a
child 515819 f5e3c3a48b5c868602e80bb079ba36eea2338897
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, RyanVM
bugs1523926
milestone66.0
Bug 1523926 - Fix open VR shmem mutex failed issue when without VR process. r=kip, a=RyanVM MozReview-Commit-ID: 5P7D75wAWI7 Differential Revision: https://phabricator.services.mozilla.com/D18301
gfx/vr/gfxVRExternal.cpp
gfx/vr/gfxVRMutex.h
gfx/vr/service/VRService.cpp
--- a/gfx/vr/gfxVRExternal.cpp
+++ b/gfx/vr/gfxVRExternal.cpp
@@ -437,41 +437,51 @@ VRSystemManagerExternal::VRSystemManager
   mShmemFD = 0;
 #elif defined(XP_WIN)
   mShmemFile = NULL;
 #elif defined(MOZ_WIDGET_ANDROID)
   mExternalStructFailed = false;
   mEnumerationCompleted = false;
 #endif
   mDoShutdown = false;
+
+#if defined(XP_WIN)
+  mMutex = CreateMutex(
+    NULL,                   // default security descriptor
+    false,                  // mutex not owned
+    TEXT("mozilla::vr::ShmemMutex"));  // object name
+
+  if (mMutex == NULL) {
+    nsAutoCString msg;
+    msg.AppendPrintf("VRSystemManagerExternal CreateMutex error \"%lu\".",
+                      GetLastError());
+    NS_WARNING(msg.get());
+    MOZ_ASSERT(false);
+    return;
+  }
+  // At xpcshell extension tests, it creates multiple VRSystemManagerExternal instances
+  // in plug-contrainer.exe. It causes GetLastError() return `ERROR_ALREADY_EXISTS`.
+  // However, even though `ERROR_ALREADY_EXISTS`, it still returns the same mutex handle.
+  //
+  // https://docs.microsoft.com/en-us/windows/desktop/api/synchapi/nf-synchapi-createmutexa
+  MOZ_ASSERT(GetLastError() == 0 || GetLastError() == ERROR_ALREADY_EXISTS);
+#endif  // defined(XP_WIN)
 }
 
-VRSystemManagerExternal::~VRSystemManagerExternal() { CloseShmem(); }
+VRSystemManagerExternal::~VRSystemManagerExternal() {
+  CloseShmem();
+#if defined(XP_WIN)
+  if (mMutex) {
+    CloseHandle(mMutex);
+    mMutex = NULL;
+  }
+#endif
+}
 
 void VRSystemManagerExternal::OpenShmem() {
-#if defined(XP_WIN)
-  if (!mMutex) {
-     mMutex = CreateMutex(
-        NULL,                   // default security descriptor
-        false,                  // mutex not owned
-        TEXT("mozilla::vr::ShmemMutex"));  // object name
-
-    if (mMutex == NULL) {
-      nsAutoCString msg("VRService CreateMutex error \"%lu\".",
-                        GetLastError());
-      NS_WARNING(msg.get());
-      MOZ_ASSERT(false);
-      return;
-    }
-    else if (GetLastError() == ERROR_ALREADY_EXISTS) {
-      NS_WARNING("CreateMutex opened an existing mutex.");
-    }
-  }
-#endif  // defined(XP_WIN)
-
   if (mExternalShmem) {
     return;
 #if defined(MOZ_WIDGET_ANDROID)
   } else if (mExternalStructFailed) {
     return;
 #endif  // defined(MOZ_WIDGET_ANDROID)
   }
 
@@ -565,22 +575,16 @@ void VRSystemManagerExternal::OpenShmem(
 
 void VRSystemManagerExternal::CheckForShutdown() {
   if (mDoShutdown) {
     Shutdown();
   }
 }
 
 void VRSystemManagerExternal::CloseShmem() {
-#if defined(XP_WIN)
-  if (mMutex) {
-    CloseHandle(mMutex);
-    mMutex = NULL;
-  }
-#endif
 #if !defined(MOZ_WIDGET_ANDROID)
   if (mSameProcess) {
     return;
   }
 #endif
 #if defined(XP_MACOSX)
   if (mExternalShmem) {
     munmap((void*)mExternalShmem, sizeof(VRExternalShmem));
--- a/gfx/vr/gfxVRMutex.h
+++ b/gfx/vr/gfxVRMutex.h
@@ -13,18 +13,19 @@ namespace gfx {
 #if defined(XP_WIN)
 class WaitForMutex
 {
 public:
   explicit WaitForMutex(HANDLE handle)
     : mHandle(handle)
     , mStatus(false) {
 
+    MOZ_ASSERT(mHandle);
+
     DWORD dwWaitResult;
-
     dwWaitResult = WaitForSingleObject(
         mHandle,    // handle to mutex
         INFINITE);  // no time-out interval
 
     switch (dwWaitResult) {
       // The thread got ownership of the mutex
       case WAIT_OBJECT_0:
         mStatus = true;
@@ -38,16 +39,20 @@ public:
       default:
         mStatus = false;
         break;
     }
   }
 
   ~WaitForMutex() {
     if (mHandle && !ReleaseMutex(mHandle)) {
+      nsAutoCString msg;
+      msg.AppendPrintf("WaitForMutex %d ReleaseMutex error \"%lu\".",
+                        mHandle, GetLastError());
+      NS_WARNING(msg.get());
       MOZ_ASSERT(false, "Failed to release mutex.");
     }
   }
 
   bool GetStatus() {
     return mStatus;
   }
 
--- a/gfx/vr/service/VRService.cpp
+++ b/gfx/vr/service/VRService.cpp
@@ -98,16 +98,37 @@ void VRService::Refresh() {
   }
 
   if (mAPIShmem->state.displayState.shutdown) {
     Stop();
   }
 }
 
 void VRService::Start() {
+#if defined(XP_WIN)
+  // Adding `!XRE_IsParentProcess()` to avoid Win 7 32-bit WebVR tests
+  // to OpenMutex when there is no GPU process to create
+  // VRSystemManagerExternal and its mutex.
+  if (!mMutex && !XRE_IsParentProcess()) {
+     mMutex = OpenMutex(
+        MUTEX_ALL_ACCESS,       // request full access
+        false,                  // handle not inheritable
+        TEXT("mozilla::vr::ShmemMutex"));  // object name
+
+    if (mMutex == NULL) {
+      nsAutoCString msg;
+      msg.AppendPrintf("VRService OpenMutex error \"%lu\".",
+                       GetLastError());
+      NS_WARNING(msg.get());
+      MOZ_ASSERT(false);
+    }
+    MOZ_ASSERT(GetLastError() == 0);
+  }
+#endif
+
   if (!mServiceThread) {
     /**
      * We must ensure that any time the service is re-started, that
      * the VRSystemState is reset, including mSystemState.enumerationCompleted
      * This must happen before VRService::Start returns to the caller, in order
      * to prevent the WebVR/WebXR promises from being resolved before the
      * enumeration has been completed.
      */
@@ -161,33 +182,16 @@ void VRService::Stop() {
   if (mMutex) {
     CloseHandle(mMutex);
   }
 #endif
   mSession = nullptr;
 }
 
 bool VRService::InitShmem() {
-#if defined(XP_WIN)
-  if (!mMutex) {
-     mMutex = OpenMutex(
-        MUTEX_ALL_ACCESS,       // request full access
-        false,                  // handle not inheritable
-        TEXT("mozilla::vr::ShmemMutex"));  // object name
-
-    if (mMutex == NULL) {
-      nsAutoCString msg("VRService OpenMutex error \"%lu\".",
-                        GetLastError());
-      NS_WARNING(msg.get());
-      MOZ_ASSERT(false);
-      return false;
-    }
-  }
-#endif
-
   if (!mVRProcessEnabled) {
     return true;
   }
 
 #if defined(XP_WIN)
   const char* kShmemName = "moz.gecko.vr_ext.0.0.1";
   base::ProcessHandle targetHandle = 0;
 
@@ -453,18 +457,20 @@ void VRService::PushState(const mozilla:
   if (pthread_mutex_lock((pthread_mutex_t*)&(mExternalShmem->systemMutex)) ==
       0) {
     memcpy((void*)&mAPIShmem->state, &aState, sizeof(VRSystemState));
     pthread_mutex_unlock((pthread_mutex_t*)&(mExternalShmem->systemMutex));
   }
 #else
   bool state = true;
 #if defined(XP_WIN)
-  WaitForMutex lock(mMutex);
-  state = lock.GetStatus();
+  if (!XRE_IsParentProcess()) {
+    WaitForMutex lock(mMutex);
+    state = lock.GetStatus();
+  }
 #endif  // defined(XP_WIN)
   if (state) {
     mAPIShmem->generationA++;
     memcpy((void*)&mAPIShmem->state, &aState, sizeof(VRSystemState));
     mAPIShmem->generationB++;
   }
 #endif // defined(MOZ_WIDGET_ANDROID)
 }
@@ -486,18 +492,20 @@ void VRService::PullState(mozilla::gfx::
   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
   bool status = true;
 #if defined(XP_WIN)
-  WaitForMutex lock(mMutex);
-  status = lock.GetStatus();
+  if (!XRE_IsParentProcess()) {
+    WaitForMutex lock(mMutex);
+    status = lock.GetStatus();
+  }
 #endif  // defined(XP_WIN)
   if (status) {
     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));