Bug 1260891 - Acquire and release the lock when destorying an `ExclusiveData<T>`'s protected value; r=terrence a=kwierso
authorNick Fitzgerald <fitzgen@gmail.com>
Thu, 31 Mar 2016 14:29:50 -0700
changeset 291113 297e38fbff063b5d7f1289be4af164b8642d727b
parent 291112 7f27f1aa002929117dda91bbe774544d844db918
child 291114 c3fb02405b68aca36fdc60e556e13e49fe5482b0
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, kwierso
bugs1260891
milestone48.0a1
Bug 1260891 - Acquire and release the lock when destorying an `ExclusiveData<T>`'s protected value; r=terrence a=kwierso Even if there is not a traditional race between uses of the protected data and the `ExclusiveData<T>`'s destruction, the thread running the destructor has no guarantee that its cache is up to date with changes already made by other threads. The new acquire and release surrounding the destructor ensures that the destructor doesn't run on stale cache lines. MozReview-Commit-ID: 1fewYlhuEIa
js/src/threading/ExclusiveData.h
--- a/js/src/threading/ExclusiveData.h
+++ b/js/src/threading/ExclusiveData.h
@@ -2,16 +2,17 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef threading_ExclusiveData_h
 #define threading_ExclusiveData_h
 
+#include "mozilla/Alignment.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Move.h"
 
 #include "threading/Mutex.h"
 
 namespace js {
 
 /**
@@ -75,39 +76,43 @@ namespace js {
  *      guard's lifetime and therefore becomes invalid. To help avoid this last
  *      foot-gun, prefer using the guard directly! Do not store raw references
  *      to the protected value in other structures!
  */
 template <typename T>
 class ExclusiveData
 {
     mutable Mutex lock_;
-    mutable T value_;
+    mutable mozilla::AlignedStorage2<T> value_;
 
     ExclusiveData(const ExclusiveData&) = delete;
     ExclusiveData& operator=(const ExclusiveData&) = delete;
 
     void acquire() const { lock_.lock(); }
     void release() const { lock_.unlock(); }
 
   public:
     /**
      * Create a new `ExclusiveData`, with perfect forwarding of the protected
      * value.
      */
     template <typename U>
-    explicit ExclusiveData(U&& u)
-      : value_(mozilla::Forward<U>(u))
-    {
+    explicit ExclusiveData(U&& u) {
+        new (value_.addr()) T(mozilla::Forward<U>(u));
     }
 
-    ExclusiveData(ExclusiveData&& rhs)
-      : value_(mozilla::Move(rhs.value_))
-    {
+    ~ExclusiveData() {
+        acquire();
+        value_.addr()->~T();
+        release();
+    }
+
+    ExclusiveData(ExclusiveData&& rhs) {
         MOZ_ASSERT(&rhs != this, "self-move disallowed!");
+        new (value_.addr()) T(mozilla::Move(*rhs.value_.addr()));
     }
 
     ExclusiveData& operator=(ExclusiveData&& rhs) {
         this->~ExclusiveData();
         new (this) ExclusiveData(mozilla::Move(rhs));
         return *this;
     }
 
@@ -143,17 +148,17 @@ class ExclusiveData
         Guard& operator=(Guard&& rhs) {
             this->~Guard();
             new (this) Guard(mozilla::Move(rhs));
             return *this;
         }
 
         T& get() const {
             MOZ_ASSERT(parent_);
-            return parent_->value_;
+            return *parent_->value_.addr();
         }
 
         operator T& () const { return get(); }
         T* operator->() const { return &get(); }
 
         ~Guard() {
             if (parent_)
                 parent_->release();