Bug 1287298 - Add an API to give up ownership of ArrayBuffer data; r=Waldo
authorEhsan Akhgari <ehsan@mozilla.com>
Sat, 16 Jul 2016 17:52:39 -0400
changeset 419829 87b7e5eac522bf5e87bcce12d5bcf50665d23442
parent 419828 7d4875921ecf575a0ddc6acfee68dc093c10a4c6
child 419830 90c19ed19a55412fee9ba0c21174aab726f6dfc7
push id31023
push usercykesiopka.bmo@gmail.com
push dateSat, 01 Oct 2016 01:30:33 +0000
reviewersWaldo
bugs1287298
milestone52.0a1
Bug 1287298 - Add an API to give up ownership of ArrayBuffer data; r=Waldo This is similar to stealing the buffer, except that the ArrayBuffer won't be detached. The caller is still responsible for freeing the buffer.
js/src/jsapi-tests/testArrayBuffer.cpp
js/src/jsapi.h
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
--- a/js/src/jsapi-tests/testArrayBuffer.cpp
+++ b/js/src/jsapi-tests/testArrayBuffer.cpp
@@ -153,8 +153,75 @@ static void GC(JSContext* cx)
 }
 
 bool hasDetachedBuffer(JS::HandleObject obj) {
     JS::RootedValue v(cx);
     return JS_GetProperty(cx, obj, "byteLength", &v) && v.toInt32() == 0;
 }
 
 END_TEST(testArrayBuffer_bug720949_viewList)
+
+BEGIN_TEST(testArrayBuffer_externalize)
+{
+    if (!testWithSize(cx, 2))    // ArrayBuffer data stored inline in the object.
+        return false;
+    if (!testWithSize(cx, 2000)) // ArrayBuffer data stored out-of-line in a separate heap allocation.
+        return false;
+
+    return true;
+}
+
+bool testWithSize(JSContext* cx, size_t n)
+{
+    JS::RootedObject buffer(cx, JS_NewArrayBuffer(cx, n));
+    CHECK(buffer != nullptr);
+
+    JS::RootedObject view(cx, JS_NewUint8ArrayWithBuffer(cx, buffer, 0, -1));
+    CHECK(view != nullptr);
+
+    void* contents = JS_ExternalizeArrayBufferContents(cx, buffer);
+    CHECK(contents != nullptr);
+    uint32_t actualLength;
+    CHECK(hasExpectedLength(cx, view, &actualLength));
+    CHECK(actualLength == n);
+    CHECK(!JS_IsDetachedArrayBufferObject(buffer));
+    CHECK(JS_GetArrayBufferByteLength(buffer) == uint32_t(n));
+
+    uint8_t* uint8Contents = static_cast<uint8_t*>(contents);
+    CHECK(*uint8Contents == 0);
+    uint8_t randomByte(rand() % UINT8_MAX);
+    *uint8Contents = randomByte;
+
+    JS::RootedValue v(cx);
+    CHECK(JS_GetElement(cx, view, 0, &v));
+    CHECK(v.toInt32() == randomByte);
+
+    view = nullptr;
+    GC(cx);
+
+    CHECK(JS_DetachArrayBuffer(cx, buffer));
+    GC(cx);
+    CHECK(*uint8Contents == randomByte);
+    JS_free(cx, contents);
+    GC(cx);
+    buffer = nullptr;
+    GC(cx);
+
+    return true;
+}
+
+static void GC(JSContext* cx)
+{
+    JS_GC(cx);
+    JS_GC(cx); // Trigger another to wait for background finalization to end
+}
+
+static bool
+hasExpectedLength(JSContext* cx, JS::HandleObject obj, uint32_t* len)
+{
+    JS::RootedValue v(cx);
+    if (!JS_GetProperty(cx, obj, "byteLength", &v))
+        return false;
+    *len = v.toInt32();
+    return true;
+}
+
+END_TEST(testArrayBuffer_externalize)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3442,16 +3442,35 @@ JS_NewArrayBufferWithExternalContents(JS
  * length set to 0 and its contents array cleared. The caller takes ownership
  * of the return value and must free it or transfer ownership via
  * JS_NewArrayBufferWithContents when done using it.
  */
 extern JS_PUBLIC_API(void*)
 JS_StealArrayBufferContents(JSContext* cx, JS::HandleObject obj);
 
 /**
+ * Returns a pointer to the ArrayBuffer |obj|'s data.  |obj| and its views will store and expose
+ * the data in the returned pointer: assigning into the returned pointer will affect values exposed
+ * by views of |obj| and vice versa.
+ *
+ * The caller must ultimately deallocate the returned pointer to avoid leaking.  The memory is
+ * *not* garbage-collected with |obj|.  These steps must be followed to deallocate:
+ *
+ * 1. The ArrayBuffer |obj| must be detached using JS_DetachArrayBuffer.
+ * 2. The returned pointer must be freed using JS_free.
+ *
+ * To perform step 1, callers *must* hold a reference to |obj| until they finish using the returned
+ * pointer.  They *must not* attempt to let |obj| be GC'd, then JS_free the pointer.
+ *
+ * If |obj| isn't an ArrayBuffer, this function returns null and reports an error.
+ */
+extern JS_PUBLIC_API(void*)
+JS_ExternalizeArrayBufferContents(JSContext* cx, JS::HandleObject obj);
+
+/**
  * Create a new mapped array buffer with the given memory mapped contents. It
  * must be legal to free the contents pointer by unmapping it. On success,
  * ownership is transferred to the new mapped array buffer.
  */
 extern JS_PUBLIC_API(JSObject*)
 JS_NewMappedArrayBufferWithContents(JSContext* cx, size_t nbytes, void* contents);
 
 /**
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -311,31 +311,31 @@ ArrayBufferObject::detach(JSContext* cx,
             MOZ_ASSERT(buffer->firstView()->is<InlineTransparentTypedObject>());
         } else {
             NoteViewBufferWasDetached(buffer->firstView(), newContents, cx);
             buffer->setFirstView(nullptr);
         }
     }
 
     if (newContents.data() != buffer->dataPointer())
-        buffer->setNewOwnedData(cx->runtime()->defaultFreeOp(), newContents);
+        buffer->setNewData(cx->runtime()->defaultFreeOp(), newContents, OwnsData);
 
     buffer->setByteLength(0);
     buffer->setIsDetached();
 }
 
 void
-ArrayBufferObject::setNewOwnedData(FreeOp* fop, BufferContents newContents)
+ArrayBufferObject::setNewData(FreeOp* fop, BufferContents newContents, OwnsState ownsState)
 {
     if (ownsData()) {
         MOZ_ASSERT(newContents.data() != dataPointer());
         releaseData(fop);
     }
 
-    setDataPointer(newContents, OwnsData);
+    setDataPointer(newContents, ownsState);
 }
 
 // This is called *only* from changeContents(), below.
 // By construction, every view parameter will be mapping unshared memory (an ArrayBuffer).
 // Hence no reason to worry about shared memory here.
 
 void
 ArrayBufferObject::changeViewContents(JSContext* cx, ArrayBufferViewObject* view,
@@ -356,24 +356,25 @@ ArrayBufferObject::changeViewContents(JS
 
     // Notify compiled jit code that the base pointer has moved.
     MarkObjectStateChange(cx, view);
 }
 
 // BufferContents is specific to ArrayBuffer, hence it will not represent shared memory.
 
 void
-ArrayBufferObject::changeContents(JSContext* cx, BufferContents newContents)
+ArrayBufferObject::changeContents(JSContext* cx, BufferContents newContents,
+                                  OwnsState ownsState)
 {
     MOZ_RELEASE_ASSERT(!isWasm());
     MOZ_ASSERT(!forInlineTypedObject());
 
     // Change buffer contents.
     uint8_t* oldDataPointer = dataPointer();
-    setNewOwnedData(cx->runtime()->defaultFreeOp(), newContents);
+    setNewData(cx->runtime()->defaultFreeOp(), newContents, ownsState);
 
     // Update all views.
     auto& innerViews = cx->compartment()->innerViews;
     if (InnerViewTable::ViewVector* views = innerViews.maybeViewsUnbarriered(this)) {
         for (size_t i = 0; i < views->length(); i++)
             changeViewContents(cx, (*views)[i], oldDataPointer, newContents);
     }
     if (firstView())
@@ -736,17 +737,17 @@ ArrayBufferObject::prepareForAsmJS(JSCon
             return false;
         }
 
         void* data = wasmBuf->dataPointer();
         memcpy(data, buffer->dataPointer(), length);
 
         // Swap the new elements into the ArrayBufferObject. Mark the
         // ArrayBufferObject so we don't do this again.
-        buffer->changeContents(cx, BufferContents::create<WASM>(data));
+        buffer->changeContents(cx, BufferContents::create<WASM>(data), OwnsData);
         buffer->setIsPreparedForAsmJS();
         MOZ_ASSERT(data == buffer->dataPointer());
         cx->zone()->updateMallocCounter(wasmBuf->mappedSize());
         return true;
     }
 
     if (!buffer->isWasm() && buffer->isPreparedForAsmJS())
         return true;
@@ -755,17 +756,17 @@ ArrayBufferObject::prepareForAsmJS(JSCon
     if (buffer->isWasm())
         return false;
 
     if (!buffer->ownsData()) {
         BufferContents contents = AllocateArrayBufferContents(cx, buffer->byteLength());
         if (!contents)
             return false;
         memcpy(contents.data(), buffer->dataPointer(), buffer->byteLength());
-        buffer->changeContents(cx, contents);
+        buffer->changeContents(cx, contents, OwnsData);
     }
 
     buffer->setIsPreparedForAsmJS();
     return true;
 }
 
 ArrayBufferObject::BufferContents
 ArrayBufferObject::createMappedContents(int fd, size_t offset, size_t length)
@@ -1092,16 +1093,42 @@ ArrayBufferObject::createDataViewForThis
 bool
 ArrayBufferObject::createDataViewForThis(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     return CallNonGenericMethod<IsArrayBuffer, createDataViewForThisImpl>(cx, args);
 }
 
 /* static */ ArrayBufferObject::BufferContents
+ArrayBufferObject::externalizeContents(JSContext* cx, Handle<ArrayBufferObject*> buffer,
+                                       bool hasStealableContents)
+{
+    MOZ_ASSERT(buffer->isPlain(), "Only support doing this on plain ABOs");
+    MOZ_ASSERT(!buffer->isDetached(), "must have contents to externalize");
+    MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents());
+
+    BufferContents contents(buffer->dataPointer(), buffer->bufferKind());
+
+    if (hasStealableContents) {
+        buffer->setOwnsData(DoesntOwnData);
+        return contents;
+    }
+
+    // Create a new chunk of memory to return since we cannot steal the
+    // existing contents away from the buffer.
+    BufferContents newContents = AllocateArrayBufferContents(cx, buffer->byteLength());
+    if (!newContents)
+        return BufferContents::createPlain(nullptr);
+    memcpy(newContents.data(), contents.data(), buffer->byteLength());
+    buffer->changeContents(cx, newContents, DoesntOwnData);
+
+    return newContents;
+}
+
+/* static */ ArrayBufferObject::BufferContents
 ArrayBufferObject::stealContents(JSContext* cx, Handle<ArrayBufferObject*> buffer,
                                  bool hasStealableContents)
 {
     // While wasm buffers cannot generally be transferred by content, the
     // stealContents() is used internally by the impl of memory growth.
     MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents() ||
                                         (buffer->isWasm() && !buffer->isPreparedForAsmJS()));
     assertSameCompartment(cx, buffer);
@@ -1651,16 +1678,48 @@ JS_FRIEND_API(JSObject*)
 js::UnwrapSharedArrayBuffer(JSObject* obj)
 {
     if (JSObject* unwrapped = CheckedUnwrap(obj))
         return unwrapped->is<SharedArrayBufferObject>() ? unwrapped : nullptr;
     return nullptr;
 }
 
 JS_PUBLIC_API(void*)
+JS_ExternalizeArrayBufferContents(JSContext* cx, HandleObject obj)
+{
+    AssertHeapIsIdle(cx);
+    CHECK_REQUEST(cx);
+    assertSameCompartment(cx, obj);
+
+    if (!obj->is<ArrayBufferObject>()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_BAD_ARGS);
+        return nullptr;
+    }
+
+    Handle<ArrayBufferObject*> buffer = obj.as<ArrayBufferObject>();
+    if (!buffer->isPlain()) {
+        // This operation isn't supported on mapped or wsm ArrayBufferObjects.
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_BAD_ARGS);
+        return nullptr;
+    }
+    if (buffer->isDetached()) {
+        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TYPED_ARRAY_DETACHED);
+        return nullptr;
+    }
+
+    // The caller assumes that a plain malloc'd buffer is returned.
+    // hasStealableContents is true for mapped buffers, so we must additionally
+    // require that the buffer is plain. In the future, we could consider
+    // returning something that handles releasing the memory.
+    bool hasStealableContents = buffer->hasStealableContents();
+
+    return ArrayBufferObject::externalizeContents(cx, buffer, hasStealableContents).data();
+}
+
+JS_PUBLIC_API(void*)
 JS_StealArrayBufferContents(JSContext* cx, HandleObject objArg)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, objArg);
 
     JSObject* obj = CheckedUnwrap(objArg);
     if (!obj)
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -271,16 +271,19 @@ class ArrayBufferObject : public ArrayBu
 
     static void copyData(Handle<ArrayBufferObject*> toBuffer,
                          Handle<ArrayBufferObject*> fromBuffer,
                          uint32_t fromIndex, uint32_t count);
 
     static void trace(JSTracer* trc, JSObject* obj);
     static void objectMoved(JSObject* obj, const JSObject* old);
 
+    static BufferContents externalizeContents(JSContext* cx,
+                                              Handle<ArrayBufferObject*> buffer,
+                                              bool hasStealableContents);
     static BufferContents stealContents(JSContext* cx,
                                         Handle<ArrayBufferObject*> buffer,
                                         bool hasStealableContents);
 
     bool hasStealableContents() const {
         // Inline elements strictly adhere to the corresponding buffer.
         return ownsData() && !isPreparedForAsmJS() && !isWasm();
     }
@@ -292,18 +295,18 @@ class ArrayBufferObject : public ArrayBu
     // later views are (weakly) stored in the compartment's InnerViewTable
     // below. Buffers usually only have one view, so this slot optimizes for
     // the common case. Avoiding entries in the InnerViewTable saves memory and
     // non-incrementalized sweep time.
     ArrayBufferViewObject* firstView();
 
     bool addView(JSContext* cx, JSObject* view);
 
-    void setNewOwnedData(FreeOp* fop, BufferContents newContents);
-    void changeContents(JSContext* cx, BufferContents newContents);
+    void setNewData(FreeOp* fop, BufferContents newContents, OwnsState ownsState);
+    void changeContents(JSContext* cx, BufferContents newContents, OwnsState ownsState);
 
     // Detach this buffer from its original memory.  (This necessarily makes
     // views of this buffer unusable for modifying that original memory.)
     static void
     detach(JSContext* cx, Handle<ArrayBufferObject*> buffer, BufferContents newContents);
 
   private:
     void changeViewContents(JSContext* cx, ArrayBufferViewObject* view,