Bug 1333429 - part 1 - remove move construction/assignment from js::Mutex; r=fitzgen
authorNathan Froyd <froydnj@mozilla.com>
Wed, 25 Jan 2017 07:36:42 -0500
changeset 331012 567ea9cd04504703c6f354b18dd02b41ae35c94f
parent 331011 50f4ccdd925f132b795df97aad4386f8c104dbc0
child 331013 e8bb22053e65e2a82456e9243a07af023a8ebb13
push id31258
push userkwierso@gmail.com
push dateThu, 26 Jan 2017 00:56:03 +0000
treeherdermozilla-central@52a34f9a6cf1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1333429
milestone54.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 1333429 - part 1 - remove move construction/assignment from js::Mutex; r=fitzgen We do this for several reasons: - Making Mutex movable requires a separate memory allocation for every Mutex, but not every Mutex client wants a movable Mutex. If movability is desired, a client can use UniquePtr<Mutex>. - Inserting movability here requires certain JS engine-specific constructs (e.g. OOM checking) that will be inconvenient if Mutexes are lifted out of the JS engine. - The related class ConditionVariable isn't movable; the related classes std::mutex and std::recursive_mutex aren't movable either.
js/src/jsapi-tests/testThreadingMutex.cpp
js/src/threading/Mutex.h
--- a/js/src/jsapi-tests/testThreadingMutex.cpp
+++ b/js/src/jsapi-tests/testThreadingMutex.cpp
@@ -29,22 +29,8 @@ END_TEST(testThreadingLockGuard)
 BEGIN_TEST(testThreadingUnlockGuard)
 {
     js::Mutex mutex(js::mutexid::TestMutex);
     js::LockGuard<js::Mutex> guard(mutex);
     js::UnlockGuard<js::Mutex> unguard(guard);
     return true;
 }
 END_TEST(testThreadingUnlockGuard)
-
-BEGIN_TEST(testThreadingMoveMutex)
-{
-    js::Mutex mutex(js::mutexid::TestMutex);
-    mutex.lock();
-    mutex.unlock();
-
-    js::Mutex another(mozilla::Move(mutex));
-    another.lock();
-    another.unlock();
-
-    return true;
-}
-END_TEST(testThreadingMoveMutex)
--- a/js/src/threading/Mutex.h
+++ b/js/src/threading/Mutex.h
@@ -25,29 +25,16 @@ namespace detail {
 class MutexImpl
 {
 public:
   struct PlatformData;
 
   MutexImpl();
   ~MutexImpl();
 
-  MutexImpl(MutexImpl&& rhs)
-    : platformData_(rhs.platformData_)
-  {
-    MOZ_ASSERT(this != &rhs, "self move disallowed!");
-    rhs.platformData_ = nullptr;
-  }
-
-  MutexImpl& operator=(MutexImpl&& rhs) {
-    this->~MutexImpl();
-    new (this) MutexImpl(mozilla::Move(rhs));
-    return *this;
-  }
-
   bool operator==(const MutexImpl& rhs) {
     return platformData_ == rhs.platformData_;
   }
 
 protected:
   void lock();
   void unlock();