Bug 1352681 - Make SAB rawBuffer refcounting for structured clone more sophisticated. r=sfink, a=lizzard
authorLars T Hansen <lhansen@mozilla.com>
Thu, 06 Apr 2017 09:04:28 +0200
changeset 379496 ac4dedf088848948275073e0ee6bbce13c8b73b5
parent 379495 c1e1e7392f5d624042df293aea5694aec95426ec
child 379497 b2ebbc981c90372f01a7d198a07c964eac6f657c
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, lizzard
bugs1352681
milestone53.0
Bug 1352681 - Make SAB rawBuffer refcounting for structured clone more sophisticated. r=sfink, a=lizzard
js/public/StructuredClone.h
js/src/vm/StructuredClone.cpp
--- a/js/public/StructuredClone.h
+++ b/js/public/StructuredClone.h
@@ -12,16 +12,17 @@
 
 #include <stdint.h>
 
 #include "jstypes.h"
 
 #include "js/RootingAPI.h"
 #include "js/TypeDecls.h"
 #include "js/Value.h"
+#include "js/Vector.h"
 
 struct JSRuntime;
 struct JSStructuredCloneReader;
 struct JSStructuredCloneWriter;
 
 // API for the HTML5 internal structured cloning algorithm.
 
 namespace JS {
@@ -183,28 +184,51 @@ struct JSStructuredCloneCallbacks {
 };
 
 enum OwnTransferablePolicy {
     OwnsTransferablesIfAny,
     IgnoreTransferablesIfAny,
     NoTransferables
 };
 
+namespace js
+{
+    class SharedArrayRawBuffer;
+
+    class SharedArrayRawBufferRefs
+    {
+      public:
+        SharedArrayRawBufferRefs() = default;
+        SharedArrayRawBufferRefs(SharedArrayRawBufferRefs&& other) = default;
+        SharedArrayRawBufferRefs& operator=(SharedArrayRawBufferRefs&& other);
+        ~SharedArrayRawBufferRefs();
+
+        MOZ_MUST_USE bool acquire(JSContext* cx, SharedArrayRawBuffer* rawbuf);
+        MOZ_MUST_USE bool acquireAll(JSContext* cx, const SharedArrayRawBufferRefs& that);
+        void takeOwnership(SharedArrayRawBufferRefs&&);
+        void releaseAll();
+
+      private:
+        js::Vector<js::SharedArrayRawBuffer*, 0, js::SystemAllocPolicy> refs_;
+    };
+}
+
 class MOZ_NON_MEMMOVABLE JSStructuredCloneData : public mozilla::BufferList<js::SystemAllocPolicy>
 {
     typedef js::SystemAllocPolicy AllocPolicy;
     typedef mozilla::BufferList<js::SystemAllocPolicy> BufferList;
 
     static const size_t kInitialSize = 0;
     static const size_t kInitialCapacity = 4096;
     static const size_t kStandardCapacity = 4096;
 
     const JSStructuredCloneCallbacks* callbacks_;
     void* closure_;
     OwnTransferablePolicy ownTransferables_;
+    js::SharedArrayRawBufferRefs refsHeld_;
 
     void setOptionalCallbacks(const JSStructuredCloneCallbacks* callbacks,
                               void* closure,
                               OwnTransferablePolicy policy) {
         callbacks_ = callbacks;
         closure_ = closure;
         ownTransferables_ = policy;
     }
@@ -273,17 +297,18 @@ class JS_PUBLIC_API(JSAutoStructuredClon
     ~JSAutoStructuredCloneBuffer() { clear(); }
 
     JSStructuredCloneData& data() { return data_; }
     bool empty() const { return !data_.Size(); }
 
     void clear(const JSStructuredCloneCallbacks* optionalCallbacks=nullptr, void* closure=nullptr);
 
     /** Copy some memory. It will be automatically freed by the destructor. */
-    bool copy(const JSStructuredCloneData& data, uint32_t version=JS_STRUCTURED_CLONE_VERSION,
+    bool copy(JSContext* cx, const JSStructuredCloneData& data,
+              uint32_t version=JS_STRUCTURED_CLONE_VERSION,
               const JSStructuredCloneCallbacks* callbacks=nullptr, void* closure=nullptr);
 
     /**
      * Adopt some memory. It will be automatically freed by the destructor.
      * data must have been allocated by the JS engine (e.g., extracted via
      * JSAutoStructuredCloneBuffer::steal).
      */
     void adopt(JSStructuredCloneData&& data, uint32_t version=JS_STRUCTURED_CLONE_VERSION,
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -222,16 +222,79 @@ struct BufferIterator {
         MOZ_ASSERT(mIter.HasRoomFor(sizeof(T)));
         return *reinterpret_cast<T*>(mIter.Data());
     }
 
     BufferList& mBuffer;
     typename BufferList::IterImpl mIter;
 };
 
+SharedArrayRawBufferRefs&
+SharedArrayRawBufferRefs::operator=(SharedArrayRawBufferRefs&& other)
+{
+    takeOwnership(Move(other));
+    return *this;
+}
+
+SharedArrayRawBufferRefs::~SharedArrayRawBufferRefs()
+{
+    releaseAll();
+}
+
+bool
+SharedArrayRawBufferRefs::acquire(JSContext* cx, SharedArrayRawBuffer* rawbuf)
+{
+    if (!refs_.append(rawbuf)) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+
+    if (!rawbuf->addReference()) {
+        refs_.popBack();
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO);
+        return false;
+    }
+
+    return true;
+}
+
+bool
+SharedArrayRawBufferRefs::acquireAll(JSContext* cx, const SharedArrayRawBufferRefs& that)
+{
+    if (!refs_.reserve(refs_.length() + that.refs_.length())) {
+        ReportOutOfMemory(cx);
+        return false;
+    }
+
+    for (auto ref : that.refs_) {
+        if (!ref->addReference()) {
+            JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO);
+            return false;
+        }
+        MOZ_ALWAYS_TRUE(refs_.append(ref));
+    }
+
+    return true;
+}
+
+void
+SharedArrayRawBufferRefs::takeOwnership(SharedArrayRawBufferRefs&& other)
+{
+    MOZ_ASSERT(refs_.empty());
+    refs_ = Move(other.refs_);
+}
+
+void
+SharedArrayRawBufferRefs::releaseAll()
+{
+    for (auto ref : refs_)
+        ref->dropReference();
+    refs_.clear();
+}
+
 struct SCOutput {
   public:
     using Iter = BufferIterator<uint64_t, TempAllocPolicy>;
 
     explicit SCOutput(JSContext* cx);
 
     JSContext* context() const { return cx; }
 
@@ -401,16 +464,19 @@ struct JSStructuredCloneWriter {
 
     bool write(HandleValue v);
 
     SCOutput& output() { return out; }
 
     bool extractBuffer(JSStructuredCloneData* data) {
         bool success = out.extractBuffer(data);
         if (success) {
+            // Move the SharedArrayRawBuf references here, SCOutput::extractBuffer
+            // moves the serialized data.
+            data->refsHeld_.takeOwnership(Move(refsHeld));
             data->setOptionalCallbacks(callbacks, closure,
                                        OwnTransferablePolicy::OwnsTransferablesIfAny);
         }
         return success;
     }
 
     JS::StructuredCloneScope cloneScope() const { return scope; }
 
@@ -479,16 +545,19 @@ struct JSStructuredCloneWriter {
     void* closure;
 
     // Set of transferable objects
     RootedValue transferable;
     Rooted<GCHashSet<JSObject*>> transferableObjects;
 
     const JS::CloneDataPolicy cloneDataPolicy;
 
+    // SharedArrayRawBuffers whose reference counts we have incremented.
+    SharedArrayRawBufferRefs refsHeld;
+
     friend bool JS_WriteString(JSStructuredCloneWriter* w, HandleString str);
     friend bool JS_WriteTypedArray(JSStructuredCloneWriter* w, HandleValue v);
     friend bool JS_ObjectNotWritten(JSStructuredCloneWriter* w, HandleObject obj);
 };
 
 JS_FRIEND_API(uint64_t)
 js::GetSCOffset(JSStructuredCloneWriter* writer)
 {
@@ -1147,22 +1216,18 @@ JSStructuredCloneWriter::writeSharedArra
         JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_NOT_CLONABLE,
                                   "SharedArrayBuffer");
         return false;
     }
 
     Rooted<SharedArrayBufferObject*> sharedArrayBuffer(context(), &CheckedUnwrap(obj)->as<SharedArrayBufferObject>());
     SharedArrayRawBuffer* rawbuf = sharedArrayBuffer->rawBufferObject();
 
-    // Avoids a race condition where the parent thread frees the buffer
-    // before the child has accepted the transferable.
-    if (!rawbuf->addReference()) {
-        JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO);
+    if (!refsHeld.acquire(context(), rawbuf))
         return false;
-    }
 
     intptr_t p = reinterpret_cast<intptr_t>(rawbuf);
     return out.writePair(SCTAG_SHARED_ARRAY_BUFFER_OBJECT, static_cast<uint32_t>(sizeof(p))) &&
            out.writeBytes(&p, sizeof(p));
 }
 
 bool
 JSStructuredCloneWriter::startObject(HandleObject obj, bool* backref)
@@ -1875,28 +1940,34 @@ JSStructuredCloneReader::readSharedArray
     SharedArrayRawBuffer* rawbuf = reinterpret_cast<SharedArrayRawBuffer*>(p);
 
     // There's no guarantee that the receiving agent has enabled shared memory
     // even if the transmitting agent has done so.  Ideally we'd check at the
     // transmission point, but that's tricky, and it will be a very rare problem
     // in any case.  Just fail at the receiving end if we can't handle it.
 
     if (!context()->compartment()->creationOptions().getSharedMemoryAndAtomicsEnabled()) {
-        // The sending side performed a reference increment before sending.
-        // Account for that here before leaving.
-        if (rawbuf)
-            rawbuf->dropReference();
-
         JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_SAB_DISABLED);
         return false;
     }
 
-    // The constructor absorbs the reference count increment performed by the sender.
+    // The new object will have a new reference to the rawbuf.
+
+    if (!rawbuf->addReference()) {
+        JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr, JSMSG_SC_SAB_REFCNT_OFLO);
+        return false;
+    }
+
     JSObject* obj = SharedArrayBufferObject::New(context(), rawbuf);
 
+    if (!obj) {
+        rawbuf->dropReference();
+        return false;
+    }
+
     vp.setObject(*obj);
     return true;
 }
 
 /*
  * Read in the data for a structured clone version 1 ArrayBuffer, performing
  * endianness-conversion while reading.
  */
@@ -2571,38 +2642,44 @@ JSAutoStructuredCloneBuffer::clear(const
 
     const JSStructuredCloneCallbacks* callbacks =
         optionalCallbacks ?  optionalCallbacks : data_.callbacks_;
     void* closure = optionalClosure ?  optionalClosure : data_.closure_;
 
     if (data_.ownTransferables_ == OwnTransferablePolicy::OwnsTransferablesIfAny)
         DiscardTransferables(data_, callbacks, closure);
     data_.ownTransferables_ = OwnTransferablePolicy::NoTransferables;
+    data_.refsHeld_.releaseAll();
     data_.Clear();
     version_ = 0;
 }
 
 bool
-JSAutoStructuredCloneBuffer::copy(const JSStructuredCloneData& srcData, uint32_t version,
-                                  const JSStructuredCloneCallbacks* callbacks,
+JSAutoStructuredCloneBuffer::copy(JSContext* cx, const JSStructuredCloneData& srcData,
+                                  uint32_t version, const JSStructuredCloneCallbacks* callbacks,
                                   void* closure)
 {
     // transferable objects cannot be copied
     if (StructuredCloneHasTransferObjects(srcData))
         return false;
 
     clear();
 
     auto iter = srcData.Iter();
     while (!iter.Done()) {
-            data_.WriteBytes(iter.Data(), iter.RemainingInSegment());
-            iter.Advance(srcData, iter.RemainingInSegment());
+        if (!data_.WriteBytes(iter.Data(), iter.RemainingInSegment()))
+            return false;
+        iter.Advance(srcData, iter.RemainingInSegment());
     }
 
     version_ = version;
+
+    if (!data_.refsHeld_.acquireAll(cx, srcData.refsHeld_))
+        return false;
+
     data_.setOptionalCallbacks(callbacks, closure, OwnTransferablePolicy::NoTransferables);
     return true;
 }
 
 void
 JSAutoStructuredCloneBuffer::adopt(JSStructuredCloneData&& data, uint32_t version,
                                    const JSStructuredCloneCallbacks* callbacks,
                                    void* closure)