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 316096 87b7e5eac522bf5e87bcce12d5bcf50665d23442
parent 316095 7d4875921ecf575a0ddc6acfee68dc093c10a4c6
child 316097 90c19ed19a55412fee9ba0c21174aab726f6dfc7
push id30759
push userphilringnalda@gmail.com
push dateSat, 01 Oct 2016 06:25:09 +0000
treeherdermozilla-central@fcc62bbf09ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1287298
milestone52.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 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,