Bug 1341496 - Part 3: Make CrossProcessSemaphore allocation fallible. r=billm, a=gchang
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 19 Apr 2017 15:39:11 +1200
changeset 396050 3eb1077706330f164be580efe5b039f3114e0cda
parent 396049 b38f7851fe6fca0dd759ed186b4986ed495b66e3
child 396051 b5526172339c2a68e1f9395b4aae7fe250bd6b33
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, gchang
bugs1341496
milestone54.0
Bug 1341496 - Part 3: Make CrossProcessSemaphore allocation fallible. r=billm, a=gchang
gfx/layers/client/TextureClient.cpp
ipc/glue/CrossProcessSemaphore.h
ipc/glue/CrossProcessSemaphore_posix.cpp
ipc/glue/CrossProcessSemaphore_unimplemented.cpp
ipc/glue/CrossProcessSemaphore_windows.cpp
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -1445,51 +1445,51 @@ public:
   mozilla::layers::ShmemSection mShmemSection;
   bool mAllocSuccess;
 };
 
 class CrossProcessSemaphoreReadLock : public TextureReadLock
 {
 public:
   CrossProcessSemaphoreReadLock()
-    : mSemaphore("TextureReadLock", 1)
+    : mSemaphore(CrossProcessSemaphore::Create("TextureReadLock", 1))
   {}
   explicit CrossProcessSemaphoreReadLock(CrossProcessSemaphoreHandle aHandle)
-    : mSemaphore(aHandle)
+    : mSemaphore(CrossProcessSemaphore::Create(aHandle))
   {}
 
   virtual bool ReadLock() override
   {
     if (!IsValid()) {
       return false;
     }
-    return mSemaphore.Wait();
+    return mSemaphore->Wait();
   }
   virtual bool TryReadLock(TimeDuration aTimeout) override
   {
     if (!IsValid()) {
       return false;
     }
-    return mSemaphore.Wait(Some(aTimeout));
+    return mSemaphore->Wait(Some(aTimeout));
   }
   virtual int32_t ReadUnlock() override
   {
     if (!IsValid()) {
       return 1;
     }
-    mSemaphore.Signal();
+    mSemaphore->Signal();
     return 1;
   }
-  virtual bool IsValid() const override { return true; }
+  virtual bool IsValid() const override { return !!mSemaphore; }
 
   virtual bool Serialize(ReadLockDescriptor& aOutput, base::ProcessId aOther) override;
 
   virtual LockType GetType() override { return TYPE_CROSS_PROCESS_SEMAPHORE; }
 
-  CrossProcessSemaphore mSemaphore;
+  UniquePtr<CrossProcessSemaphore> mSemaphore;
 };
 
 // static
 already_AddRefed<TextureReadLock>
 TextureReadLock::Deserialize(const ReadLockDescriptor& aDescriptor, ISurfaceAllocator* aAllocator)
 {
   switch (aDescriptor.type()) {
     case ReadLockDescriptor::TShmemSection: {
@@ -1662,17 +1662,17 @@ ShmemTextureReadLock::GetReadCount() {
   ShmReadLockInfo* info = GetShmReadLockInfoPtr();
   return info->readCount;
 }
 
 bool
 CrossProcessSemaphoreReadLock::Serialize(ReadLockDescriptor& aOutput, base::ProcessId aOther)
 {
   if (IsValid()) {
-    aOutput = ReadLockDescriptor(CrossProcessSemaphoreDescriptor(mSemaphore.ShareToProcess(aOther)));
+    aOutput = ReadLockDescriptor(CrossProcessSemaphoreDescriptor(mSemaphore->ShareToProcess(aOther)));
     return true;
   } else {
     return false;
   }
 }
 
 void
 TextureClient::EnableBlockingReadLock()
--- a/ipc/glue/CrossProcessSemaphore.h
+++ b/ipc/glue/CrossProcessSemaphore.h
@@ -40,23 +40,24 @@ typedef uintptr_t CrossProcessSemaphoreH
 
 class CrossProcessSemaphore
 {
 public:
   /**
    * CrossProcessSemaphore
    * @param name A name which can reference this lock (currently unused)
    **/
-  explicit CrossProcessSemaphore(const char* aName, uint32_t aInitialValue);
+  static CrossProcessSemaphore* Create(const char* aName, uint32_t aInitialValue);
+
   /**
    * CrossProcessSemaphore
    * @param handle A handle of an existing cross process semaphore that can be
    *               opened.
    */
-  explicit CrossProcessSemaphore(CrossProcessSemaphoreHandle aHandle);
+  static CrossProcessSemaphore* Create(CrossProcessSemaphoreHandle aHandle);
 
   ~CrossProcessSemaphore();
 
   /**
    * Decrements the current value of the semaphore. This will block if the
    * current value of the semaphore is 0, up to a maximum of aWaitTime (if
    * specified).
    * Returns true if the semaphore was succesfully decremented, false otherwise.
@@ -80,16 +81,18 @@ public:
 private:
   friend struct IPC::ParamTraits<CrossProcessSemaphore>;
 
   CrossProcessSemaphore();
   CrossProcessSemaphore(const CrossProcessSemaphore&);
   CrossProcessSemaphore &operator=(const CrossProcessSemaphore&);
 
 #if defined(OS_WIN)
+  explicit CrossProcessSemaphore(HANDLE aSemaphore);
+
   HANDLE mSemaphore;
 #elif !defined(OS_MACOSX)
   RefPtr<mozilla::ipc::SharedMemoryBasic> mSharedBuffer;
   sem_t* mSemaphore;
   mozilla::Atomic<int32_t>* mRefCount;
 #endif
 };
 
--- a/ipc/glue/CrossProcessSemaphore_posix.cpp
+++ b/ipc/glue/CrossProcessSemaphore_posix.cpp
@@ -21,83 +21,94 @@ struct SemaphoreData {
   mozilla::Atomic<int32_t> mRefCount;
   uint32_t mInitialValue;
 };
 
 } // namespace
 
 namespace mozilla {
 
-CrossProcessSemaphore::CrossProcessSemaphore(const char*, uint32_t aInitialValue)
-    : mSemaphore(nullptr)
-    , mRefCount(nullptr)
+/* static */ CrossProcessSemaphore*
+CrossProcessSemaphore::Create(const char*, uint32_t aInitialValue)
 {
-  mSharedBuffer = new ipc::SharedMemoryBasic;
-  if (!mSharedBuffer->Create(sizeof(SemaphoreData))) {
-    MOZ_CRASH();
-  }
-
-  if (!mSharedBuffer->Map(sizeof(SemaphoreData))) {
-    MOZ_CRASH();
+  RefPtr<ipc::SharedMemoryBasic> sharedBuffer = new ipc::SharedMemoryBasic;
+  if (!sharedBuffer->Create(sizeof(SemaphoreData))) {
+    return nullptr;
   }
 
-  SemaphoreData* data = static_cast<SemaphoreData*>(mSharedBuffer->memory());
-
-  if (!data) {
-    MOZ_CRASH();
+  if (!sharedBuffer->Map(sizeof(SemaphoreData))) {
+    return nullptr;
   }
 
-  data->mInitialValue = aInitialValue;
-  mSemaphore = &data->mSemaphore;
-  mRefCount = &data->mRefCount;
+  SemaphoreData* data = static_cast<SemaphoreData*>(sharedBuffer->memory());
 
-  *mRefCount = 1;
-  if (sem_init(mSemaphore, 1, data->mInitialValue)) {
-    MOZ_CRASH();
+  if (!data) {
+    return nullptr;
+  }
+
+  if (sem_init(&data->mSemaphore, 1, aInitialValue)) {
+    return nullptr;
   }
 
-  MOZ_COUNT_CTOR(CrossProcessSemaphore);
+  CrossProcessSemaphore* sem = new CrossProcessSemaphore;
+  sem->mSharedBuffer = sharedBuffer;
+  sem->mSemaphore = &data->mSemaphore;
+  sem->mRefCount = &data->mRefCount;
+  *sem->mRefCount = 1;
+
+  data->mInitialValue = aInitialValue;
+
+  return sem;
 }
 
-CrossProcessSemaphore::CrossProcessSemaphore(CrossProcessSemaphoreHandle aHandle)
-    : mSemaphore(nullptr)
-    , mRefCount(nullptr)
+/* static */ CrossProcessSemaphore*
+CrossProcessSemaphore::Create(CrossProcessSemaphoreHandle aHandle)
 {
-  mSharedBuffer = new ipc::SharedMemoryBasic;
+  RefPtr<ipc::SharedMemoryBasic> sharedBuffer = new ipc::SharedMemoryBasic;
 
-  if (!mSharedBuffer->IsHandleValid(aHandle)) {
-    MOZ_CRASH();
+  if (!sharedBuffer->IsHandleValid(aHandle)) {
+    return nullptr;
   }
 
-  if (!mSharedBuffer->SetHandle(aHandle)) {
-    MOZ_CRASH();
+  if (!sharedBuffer->SetHandle(aHandle)) {
+    return nullptr;
   }
 
-  if (!mSharedBuffer->Map(sizeof(SemaphoreData))) {
-    MOZ_CRASH();
+  if (!sharedBuffer->Map(sizeof(SemaphoreData))) {
+    return nullptr;
   }
 
-  SemaphoreData* data = static_cast<SemaphoreData*>(mSharedBuffer->memory());
+  SemaphoreData* data = static_cast<SemaphoreData*>(sharedBuffer->memory());
 
   if (!data) {
-    MOZ_CRASH();
+    return nullptr;
   }
 
-  mSemaphore = &data->mSemaphore;
-  mRefCount = &data->mRefCount;
-  int32_t oldCount = (*mRefCount)++;
-
+  int32_t oldCount = data->mRefCount++;
   if (oldCount == 0) {
     // The other side has already let go of their CrossProcessSemaphore, so now
     // mSemaphore is garbage. We need to re-initialize it.
-    if (sem_init(mSemaphore, 1, data->mInitialValue)) {
-      MOZ_CRASH();
+    if (sem_init(&data->mSemaphore, 1, data->mInitialValue)) {
+      data->mRefCount--;
+      return nullptr;
     }
   }
 
+  CrossProcessSemaphore* sem = new CrossProcessSemaphore;
+  sem->mSharedBuffer = sharedBuffer;
+  sem->mSemaphore = &data->mSemaphore;
+  sem->mRefCount = &data->mRefCount;
+  return sem;
+}
+
+
+CrossProcessSemaphore::CrossProcessSemaphore()
+  : mSemaphore(nullptr)
+  , mRefCount(nullptr)
+{
   MOZ_COUNT_CTOR(CrossProcessSemaphore);
 }
 
 CrossProcessSemaphore::~CrossProcessSemaphore()
 {
   int32_t oldCount = --(*mRefCount);
 
   if (oldCount == 0) {
--- a/ipc/glue/CrossProcessSemaphore_unimplemented.cpp
+++ b/ipc/glue/CrossProcessSemaphore_unimplemented.cpp
@@ -5,24 +5,33 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CrossProcessSemaphore.h"
 
 #include "nsDebug.h"
 
 namespace mozilla {
 
-CrossProcessSemaphore::CrossProcessSemaphore(const char*, uint32_t)
+/* static */ CrossProcessSemaphore*
+CrossProcessSemaphore::Create(const char*, uint32_t)
 {
   MOZ_CRASH("Cross-process semaphores not allowed on this platform.");
+  return nullptr;
 }
 
-CrossProcessSemaphore::CrossProcessSemaphore(CrossProcessSemaphoreHandle)
+/* static */ CrossProcessSemaphore*
+CrossProcessSemaphore::Create(CrossProcessSemaphoreHandle)
 {
   MOZ_CRASH("Cross-process semaphores not allowed on this platform.");
+  return nullptr;
+}
+
+CrossProcessSemaphore::CrossProcessSemaphore()
+{
+  MOZ_CRASH("Cross-process semaphores not allowed on this platform - woah! We should've aborted by now!");
 }
 
 CrossProcessSemaphore::~CrossProcessSemaphore()
 {
   MOZ_CRASH("Cross-process semaphores not allowed on this platform - woah! We should've aborted by now!");
 }
 
 bool
--- a/ipc/glue/CrossProcessSemaphore_windows.cpp
+++ b/ipc/glue/CrossProcessSemaphore_windows.cpp
@@ -12,35 +12,43 @@
 #include "nsISupportsImpl.h"
 #include "ProtocolUtils.h"
 
 using base::GetCurrentProcessHandle;
 using base::ProcessHandle;
 
 namespace mozilla {
 
-CrossProcessSemaphore::CrossProcessSemaphore(const char*, uint32_t aInitialValue)
+/* static */ CrossProcessSemaphore*
+CrossProcessSemaphore::Create(const char*, uint32_t aInitialValue)
 {
   // We explicitly share this using DuplicateHandle, we do -not- want this to
   // be inherited by child processes by default! So no security attributes are
   // given.
-  mSemaphore = ::CreateSemaphoreA(nullptr, aInitialValue, 0x7fffffff, nullptr);
-  if (!mSemaphore) {
-    MOZ_CRASH("This shouldn't happen - failed to create semaphore!");
+  HANDLE semaphore = ::CreateSemaphoreA(nullptr, aInitialValue, 0x7fffffff, nullptr);
+  if (!semaphore) {
+    return nullptr;
   }
-  MOZ_COUNT_CTOR(CrossProcessSemaphore);
+  return new CrossProcessSemaphore(semaphore);
 }
 
-CrossProcessSemaphore::CrossProcessSemaphore(CrossProcessSemaphoreHandle aHandle)
+/* static */ CrossProcessSemaphore*
+CrossProcessSemaphore::Create(CrossProcessSemaphoreHandle aHandle)
 {
   DWORD flags;
   if (!::GetHandleInformation(aHandle, &flags)) {
-    MOZ_CRASH("Attempt to construct a semaphore from an invalid handle!");
+    return nullptr;
   }
-  mSemaphore = aHandle;
+
+  return new CrossProcessSemaphore(aHandle);
+}
+
+CrossProcessSemaphore::CrossProcessSemaphore(HANDLE aSemaphore)
+  : mSemaphore(aSemaphore)
+{
   MOZ_COUNT_CTOR(CrossProcessSemaphore);
 }
 
 CrossProcessSemaphore::~CrossProcessSemaphore()
 {
   MOZ_ASSERT(mSemaphore, "Improper construction of semaphore or double free.");
   ::CloseHandle(mSemaphore);
   MOZ_COUNT_DTOR(CrossProcessSemaphore);