Bug 1523926 - Fix open VR shmem mutex failed issue when without VR process. r=kip
☠☠ backed out by 1206de2a542c ☠ ☠
authorDaosheng Mu <daoshengmu@gmail.com>
Sun, 03 Feb 2019 07:19:58 +0000
changeset 456579 12b2328eedd1c1d8863a4ee2c18591eaf2a522dd
parent 456578 5f006a3c652c3a5def8800b9b0e65c4213b86785
child 456582 1b713c9f40d5817de78f88ac3414cc90bf7531f4
push id35490
push useraiakab@mozilla.com
push dateSun, 03 Feb 2019 09:50:26 +0000
treeherdermozilla-central@12b2328eedd1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskip
bugs1523926
milestone67.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 1523926 - Fix open VR shmem mutex failed issue when without VR process. r=kip 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,34 @@ void VRService::Refresh() {
   }
 
   if (mAPIShmem->state.displayState.shutdown) {
     Stop();
   }
 }
 
 void VRService::Start() {
+#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;
+      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 +179,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;