Bug 829909 - Fix some Plugin Hang UI synchronization problems. r=bbondy
authorAaron Klotz <aklotz@mozilla.com>
Thu, 17 Jan 2013 21:22:21 -0500
changeset 119228 49cfb1e213e65a24da6d121a9e9759bba4c58046
parent 119227 93794651309a89b0bb8ec6f7a9cfb72c970c4b14
child 119229 765fb43a17a06c355e06f67e3dd2ce83368a130f
push id24195
push userMs2ger@gmail.com
push dateSat, 19 Jan 2013 16:10:11 +0000
treeherdermozilla-central@02e12a80aef9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbondy
bugs829909
milestone21.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 829909 - Fix some Plugin Hang UI synchronization problems. r=bbondy
dom/plugins/ipc/MiniShmParent.cpp
dom/plugins/ipc/PluginHangUIParent.cpp
dom/plugins/ipc/hangui/MiniShmBase.h
dom/plugins/ipc/hangui/MiniShmChild.cpp
--- a/dom/plugins/ipc/MiniShmParent.cpp
+++ b/dom/plugins/ipc/MiniShmParent.cpp
@@ -121,16 +121,18 @@ MiniShmParent::Init(MiniShmObserver* aOb
   ScopedMappedFileView view(::MapViewOfFile(mapping,
                                             FILE_MAP_WRITE,
                                             0, 0, 0));
   if (!view.IsValid()) {
     return NS_ERROR_FAILURE;
   }
   nsresult rv = SetView(view, aSectionSize, false);
   NS_ENSURE_SUCCESS(rv, rv);
+  rv = SetGuard(childGuard, aTimeout);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   MiniShmInit* initStruct = nullptr;
   rv = GetWritePtrInternal(initStruct);
   NS_ENSURE_SUCCESS(rv, rv);
   initStruct->mParentEvent = parentEvent;
   initStruct->mParentGuard = parentGuard;
   initStruct->mChildEvent = childEvent;
   initStruct->mChildGuard = childGuard;
@@ -169,22 +171,19 @@ MiniShmParent::GetCookie(std::wstring& c
   }
   cookie = oss.str();
   return NS_OK;
 }
 
 nsresult
 MiniShmParent::Send()
 {
-  if (!mChildEvent || !mChildGuard) {
+  if (!mChildEvent) {
     return NS_ERROR_NOT_INITIALIZED;
   }
-  if (::WaitForSingleObject(mChildGuard, mTimeout) != WAIT_OBJECT_0) {
-    return NS_ERROR_FAILURE;
-  }
   if (!::SetEvent(mChildEvent)) {
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
 bool
 MiniShmParent::IsConnected() const
--- a/dom/plugins/ipc/PluginHangUIParent.cpp
+++ b/dom/plugins/ipc/PluginHangUIParent.cpp
@@ -317,16 +317,22 @@ PluginHangUIParent::GetHangUIOwnerWindow
 void
 PluginHangUIParent::OnMiniShmEvent(MiniShmBase *aMiniShmObj)
 {
   const PluginHangUIResponse* response = nullptr;
   nsresult rv = aMiniShmObj->GetReadPtr(response);
   NS_ASSERTION(NS_SUCCEEDED(rv),
                "Couldn't obtain read pointer OnMiniShmEvent");
   if (NS_SUCCEEDED(rv)) {
+    // The child process has returned a response so we shouldn't worry about 
+    // its state anymore.
+    if (::UnregisterWaitEx(mRegWait, NULL)) {
+      mRegWait = NULL;
+    }
+
     RecvUserResponse(response->mResponseBits);
   }
 }
 
 void
 PluginHangUIParent::OnMiniShmConnect(MiniShmBase* aMiniShmObj)
 {
   PluginHangUICommand* cmd = nullptr;
--- a/dom/plugins/ipc/hangui/MiniShmBase.h
+++ b/dom/plugins/ipc/hangui/MiniShmBase.h
@@ -116,27 +116,31 @@ public:
    * other types used by the protocol being implemented.
    *
    * @param aPtr Pointer to receive the shared memory address.
    *             This value is set if and only if the function 
    *             succeeded.
    * @return NS_OK if and only if aPtr was successfully obtained.
    *         NS_ERROR_ILLEGAL_VALUE if type T is not valid for MiniShm.
    *         NS_ERROR_NOT_INITIALIZED if there is no valid MiniShm connection.
+   *         NS_ERROR_NOT_AVAILABLE if the memory is not safe to write.
    */
   template<typename T> nsresult
   GetWritePtr(T*& aPtr)
   {
-    if (!mWriteHeader) {
+    if (!mWriteHeader || !mGuard) {
       return NS_ERROR_NOT_INITIALIZED;
     }
     if (sizeof(T) > mPayloadMaxLen ||
         T::identifier <= RESERVED_CODE_LAST) {
       return NS_ERROR_ILLEGAL_VALUE;
     }
+    if (::WaitForSingleObject(mGuard, mTimeout) != WAIT_OBJECT_0) {
+      return NS_ERROR_NOT_AVAILABLE;
+    }
     mWriteHeader->mId = T::identifier;
     mWriteHeader->mPayloadLen = sizeof(T);
     aPtr = reinterpret_cast<T*>(mWriteHeader + 1);
     return NS_OK;
   }
 
   /**
    * Obtains a readable pointer into shared memory of type T.
@@ -214,17 +218,19 @@ protected:
     };
     bool      mSucceeded;
   };
 
   MiniShmBase()
     : mObserver(nullptr),
       mWriteHeader(nullptr),
       mReadHeader(nullptr),
-      mPayloadMaxLen(0)
+      mPayloadMaxLen(0),
+      mGuard(NULL),
+      mTimeout(INFINITE)
   {
   }
   virtual ~MiniShmBase()
   { }
 
   virtual void
   OnEvent()
   {
@@ -256,16 +262,27 @@ protected:
       mWriteHeader = static_cast<MiniShmHeader*>(aView);
       mReadHeader = reinterpret_cast<MiniShmHeader*>(static_cast<char*>(aView)
                                                      + aSize / 2U);
     }
     mPayloadMaxLen = aSize / 2U - sizeof(MiniShmHeader);
     return NS_OK;
   }
 
+  nsresult
+  SetGuard(HANDLE aGuard, DWORD aTimeout)
+  {
+    if (!aGuard || !aTimeout) {
+      return NS_ERROR_ILLEGAL_VALUE;
+    }
+    mGuard = aGuard;
+    mTimeout = aTimeout;
+    return NS_OK;
+  }
+
   inline void
   SetObserver(MiniShmObserver *aObserver) { mObserver = aObserver; }
 
   /**
    * Obtains a writable pointer into shared memory of type T. This version 
    * differs from GetWritePtr in that it allows typename T to be one of 
    * the private data structures declared in MiniShmBase.
    *
@@ -299,16 +316,18 @@ protected:
     object->OnEvent();
   }
 
 private:
   MiniShmObserver*  mObserver;
   MiniShmHeader*    mWriteHeader;
   MiniShmHeader*    mReadHeader;
   unsigned int      mPayloadMaxLen;
+  HANDLE            mGuard;
+  DWORD             mTimeout;
 
   DISALLOW_COPY_AND_ASSIGN(MiniShmBase);
 };
 
 } // namespace plugins
 } // namespace mozilla
 
 #endif // mozilla_plugins_MiniShmBase_h
--- a/dom/plugins/ipc/hangui/MiniShmChild.cpp
+++ b/dom/plugins/ipc/hangui/MiniShmChild.cpp
@@ -23,22 +23,24 @@ MiniShmChild::MiniShmChild()
     mTimeout(INFINITE)
 {}
 
 MiniShmChild::~MiniShmChild()
 {
   if (mRegWait) {
     ::UnregisterWaitEx(mRegWait, INVALID_HANDLE_VALUE);
   }
+  if (mParentGuard) {
+    // Try to avoid shutting down while the parent's event handler is running.
+    ::WaitForSingleObject(mParentGuard, mTimeout);
+    ::CloseHandle(mParentGuard);
+  }
   if (mParentEvent) {
     ::CloseHandle(mParentEvent);
   }
-  if (mParentGuard) {
-    ::CloseHandle(mParentGuard);
-  }
   if (mChildEvent) {
     ::CloseHandle(mChildEvent);
   }
   if (mChildGuard) {
     ::CloseHandle(mChildGuard);
   }
   if (mView) {
     ::UnmapViewOfFile(mView);
@@ -90,16 +92,20 @@ MiniShmChild::Init(MiniShmObserver* aObs
   rv = GetReadPtr(initStruct);
   if (NS_FAILED(rv)) {
     return rv;
   }
   if (!initStruct->mParentEvent || !initStruct->mParentGuard ||
       !initStruct->mChildEvent || !initStruct->mChildGuard) {
     return NS_ERROR_FAILURE;
   }
+  rv = SetGuard(initStruct->mParentGuard, aTimeout);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   if (!::RegisterWaitForSingleObject(&mRegWait,
                                      initStruct->mChildEvent,
                                      &SOnEvent,
                                      this,
                                      INFINITE,
                                      WT_EXECUTEDEFAULT)) {
     return NS_ERROR_FAILURE;
   }
@@ -141,22 +147,19 @@ MiniShmChild::Init(MiniShmObserver* aObs
 
   OnConnect();
   return NS_OK;
 }
 
 nsresult
 MiniShmChild::Send()
 {
-  if (!mParentEvent || !mParentGuard) {
+  if (!mParentEvent) {
     return NS_ERROR_NOT_INITIALIZED;
   }
-  if (::WaitForSingleObject(mParentGuard, mTimeout) != WAIT_OBJECT_0) {
-    return NS_ERROR_FAILURE;
-  }
   if (!::SetEvent(mParentEvent)) {
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
 void
 MiniShmChild::OnEvent()