Bug 1430438 - Allow reference counted data in JS_NewArrayBufferWithContents(). r=sfink

--HG-- extra : rebase_source : fbea1f2f9408247b2375c575ca2d8997d43d7200
2018-02-22 21:03:00 -08:00 · 2018-02-22 21:03:00 -08:00 · 5dd1b01d13
--- a/js/src/jsapi-tests/testArrayBuffer.cpp
+++ b/js/src/jsapi-tests/testArrayBuffer.cpp
@ -225,3 +225,118 @@ hasExpectedLength(JSContext* cx, JS::HandleObject obj, uint32_t* len)
 }

 END_TEST(testArrayBuffer_externalize)
+
+BEGIN_TEST(testArrayBuffer_refcountedContents)
+{
+    RefCountedData data("One two three four");
+    JS::RootedObject buffer(cx, JS_NewExternalArrayBuffer(cx, data.len(), data.contents(),
+        &RefCountedData::incCallback, &RefCountedData::decCallback, &data));
+    CHECK(buffer);
+    CHECK_EQUAL(data.refcount(), size_t(2));
+
+    uint32_t len;
+    bool isShared;
+    uint8_t* bufferData;
+    js::GetArrayBufferLengthAndData(buffer, &len, &isShared, &bufferData);
+    CHECK_EQUAL(len, data.len());
+    CHECK(bufferData == data.contents());
+    CHECK(strcmp(reinterpret_cast<char*>(bufferData), data.asString()) == 0);
+
+    buffer = nullptr;
+    JS_GC(cx);
+    JS_GC(cx);
+    CHECK_EQUAL(data.refcount(), size_t(1));
+
+    data.decref();
+    CHECK_NULL(data.contents());
+    CHECK_EQUAL(data.refcount(), size_t(0));
+
+    return true;
+}
+END_TEST(testArrayBuffer_refcountedContents)
+
+BEGIN_TEST(testArrayBuffer_customFreeFunc)
+{
+    RefCountedData data("One two three four");
+    // Without passing a ref function, the buffer takes over the one existing
+    // reference to the data.
+    JS::RootedObject buffer(cx, JS_NewExternalArrayBuffer(cx, data.len(), data.contents(),
+        nullptr, &RefCountedData::decCallback, &data));
+    CHECK(buffer);
+    CHECK_EQUAL(data.refcount(), size_t(1));
+
+    uint32_t len;
+    bool isShared;
+    uint8_t* bufferData;
+    js::GetArrayBufferLengthAndData(buffer, &len, &isShared, &bufferData);
+    CHECK_EQUAL(len, data.len());
+    CHECK(bufferData == data.contents());
+    CHECK(strcmp(reinterpret_cast<char*>(bufferData), data.asString()) == 0);
+
+    buffer = nullptr;
+    JS_GC(cx);
+    JS_GC(cx);
+    CHECK_NULL(data.contents());
+    CHECK_EQUAL(data.refcount(), size_t(0));
+
+    return true;
+}
+END_TEST(testArrayBuffer_customFreeFunc)
+
+BEGIN_TEST(testArrayBuffer_staticContents)
+{
+    RefCountedData data("One two three four");
+    // When passing neither a ref nor unref function, the buffer doesn't own
+    // any reference.
+    JS::RootedObject buffer(cx, JS_NewExternalArrayBuffer(cx, data.len(), data.contents(),
+        nullptr, nullptr));
+    CHECK(buffer);
+    CHECK_EQUAL(data.refcount(), size_t(1));
+
+    uint32_t len;
+    bool isShared;
+    uint8_t* bufferData;
+    js::GetArrayBufferLengthAndData(buffer, &len, &isShared, &bufferData);
+    CHECK_EQUAL(len, data.len());
+    CHECK(bufferData == data.contents());
+    CHECK(strcmp(reinterpret_cast<char*>(bufferData), data.asString()) == 0);
+
+    buffer = nullptr;
+    JS_GC(cx);
+    JS_GC(cx);
+    CHECK_EQUAL(data.refcount(), size_t(1));
+
+    data.decref();
+    return true;
+}
+END_TEST(testArrayBuffer_staticContents)
+
+BEGIN_TEST(testArrayBuffer_stealDetachExternal)
+{
+    RefCountedData data("One two three four");
+    JS::RootedObject buffer(cx, JS_NewExternalArrayBuffer(cx, data.len(), data.contents(),
+        &RefCountedData::incCallback, &RefCountedData::decCallback, &data));
+    CHECK(buffer);
+    data.decref();
+    CHECK_EQUAL(data.refcount(), size_t(1));
+
+    void* stolenContents = JS_StealArrayBufferContents(cx, buffer);
+    // External buffers are currently not stealable, since stealing only
+    // gives you a pointer with no indication how to free it. So this should
+    // copy the data.
+    CHECK(stolenContents != data.contents());
+    CHECK(strcmp(reinterpret_cast<char*>(stolenContents), data.asString()) == 0);
+    // External buffers are currently not stealable, so this should keep the
+    // reference to the data and just mark the buffer as detached.
+    CHECK(JS_IsDetachedArrayBufferObject(buffer));
+    CHECK_EQUAL(data.refcount(), size_t(1));
+
+    buffer = nullptr;
+    JS_GC(cx);
+    JS_GC(cx);
+    CHECK_NULL(data.contents());
+    CHECK_EQUAL(data.refcount(), size_t(0));
+
+    return true;
+}
+END_TEST(testArrayBuffer_stealDetachExternal)
--- a/js/src/jsapi-tests/testStructuredClone.cpp
+++ b/js/src/jsapi-tests/testStructuredClone.cpp
@ -82,6 +82,62 @@ BEGIN_TEST(testStructuredClone_string)
 }
 END_TEST(testStructuredClone_string)

+BEGIN_TEST(testStructuredClone_externalArrayBuffer)
+{
+    RefCountedData data("One two three four");
+    JS::RootedObject g1(cx, createGlobal());
+    JS::RootedObject g2(cx, createGlobal());
+    CHECK(g1);
+    CHECK(g2);
+
+    JS::RootedValue v1(cx);
+
+    {
+        JSAutoCompartment ac(cx, g1);
+
+        JS::RootedObject obj(cx, JS_NewExternalArrayBuffer(cx, data.len(), data.contents(),
+            &RefCountedData::incCallback, &RefCountedData::decCallback, &data));
+        data.decref();
+        CHECK_EQUAL(data.refcount(), size_t(1));
+
+        v1 = JS::ObjectOrNullValue(obj);
+        CHECK(v1.isObject());
+    }
+
+    {
+        JSAutoCompartment ac(cx, g2);
+        JS::RootedValue v2(cx);
+
+        CHECK(JS_StructuredClone(cx, v1, &v2, nullptr, nullptr));
+        CHECK(v2.isObject());
+
+        JS::RootedObject obj(cx, &v2.toObject());
+        CHECK(&v1.toObject() != obj);
+
+        uint32_t len;
+        bool isShared;
+        uint8_t* clonedData;
+        js::GetArrayBufferLengthAndData(obj, &len, &isShared, &clonedData);
+
+        // The contents of the two array buffers should be equal, but not the
+        // same pointer, and an extra reference should not be taken.
+        CHECK_EQUAL(len, data.len());
+        CHECK(clonedData != data.contents());
+        CHECK(strcmp(reinterpret_cast<char*>(clonedData), data.asString()) == 0);
+        CHECK_EQUAL(data.refcount(), size_t(1));
+    }
+
+    // GC the array buffer before data goes out of scope
+    v1.setNull();
+    JS_GC(cx);
+    JS_GC(cx); // Trigger another to wait for background finalization to end
+
+    CHECK_EQUAL(data.refcount(), size_t(0));
+
+    return true;
+}
+END_TEST(testStructuredClone_externalArrayBuffer)
+
 struct StructuredCloneTestPrincipals final : public JSPrincipals {
    uint32_t rank;

--- a/js/src/jsapi-tests/tests.h
+++ b/js/src/jsapi-tests/tests.h
@ -441,6 +441,43 @@ class TestJSPrincipals : public JSPrincipals
    }
 };

+// A class that simulates refcounted data, for testing with array buffers.
+class RefCountedData {
+    char* contents_;
+    size_t len_;
+    size_t refcount_;
+
+  public:
+    explicit RefCountedData(const char* str) : contents_(strdup(str)),
+        len_(strlen(str) + 1), refcount_(1) { }
+
+    size_t len() const { return len_; }
+    void* contents() const { return contents_; }
+    char* asString() const { return contents_; }
+    size_t refcount() const { return refcount_; }
+
+    void incref() { refcount_++; }
+    void decref() {
+        refcount_--;
+        if (refcount_ == 0) {
+            free(contents_);
+            contents_ = nullptr;
+        }
+    }
+
+    static void incCallback(void* contents, void* userData) {
+        auto self = static_cast<RefCountedData*>(userData);
+        MOZ_ASSERT(self->contents() == contents);
+        self->incref();
+    }
+
+    static void decCallback(void* contents, void* userData) {
+        auto self = static_cast<RefCountedData*>(userData);
+        MOZ_ASSERT(self->contents() == contents);
+        self->decref();
+    }
+};
+
 #ifdef JS_GC_ZEAL
 /*
 * Temporarily disable the GC zeal setting. This is only useful in tests that
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@ -3253,6 +3253,42 @@ JS_SetAllNonReservedSlotsToUndefined(JSContext* cx, JSObject* objArg);
 extern JS_PUBLIC_API(JSObject*)
 JS_NewArrayBufferWithContents(JSContext* cx, size_t nbytes, void* contents);

+namespace JS {
+
+using BufferContentsRefFunc = void (*)(void* contents, void* userData);
+
+}  /* namespace JS */
+
+/**
+ * Create a new array buffer with the given contents. The ref and unref
+ * functions should increment or decrement the reference count of the contents.
+ * These functions allow array buffers to be used with embedder objects that
+ * use reference counting, for example. The contents must not be modified by
+ * any reference holders, internal or external.
+ *
+ * On success, the new array buffer takes a reference, and |ref(contents,
+ * refUserData)| will be called. When the array buffer is ready to be disposed
+ * of, |unref(contents, refUserData)| will be called to release the array
+ * buffer's reference on the contents.
+ *
+ * The ref and unref functions must not call any JSAPI functions that could
+ * cause a garbage collection.
+ *
+ * The ref function is optional. If it is nullptr, the caller is responsible
+ * for incrementing the reference count before passing the contents to this
+ * function. This also allows using non-reference-counted contents that must be
+ * freed with some function other than free().
+ *
+ * The ref function may also be called in case the buffer is cloned in some
+ * way. Currently this is not used, but it may be in the future. If the ref
+ * function is nullptr, any operation where an extra reference would otherwise
+ * be taken, will either copy the data, or throw an exception.
+ */
+extern JS_PUBLIC_API(JSObject*)
+JS_NewExternalArrayBuffer(JSContext* cx, size_t nbytes, void* contents,
+                          JS::BufferContentsRefFunc ref, JS::BufferContentsRefFunc unref,
+                          void* refUserData = nullptr);
+
 /**
 * Create a new array buffer with the given contents.  The array buffer does not take ownership of
 * contents, and JS_DetachArrayBuffer must be called before the contents are disposed of.
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@ -964,6 +964,13 @@ ArrayBufferObject::dataPointerShared() const
    return SharedMem<uint8_t*>::unshared(getSlot(DATA_SLOT).toPrivate());
 }

+ArrayBufferObject::RefcountInfo*
+ArrayBufferObject::refcountInfo() const
+{
+    MOZ_ASSERT(isExternal());
+    return reinterpret_cast<RefcountInfo*>(inlineDataPointer());
+}
+
 void
 ArrayBufferObject::releaseData(FreeOp* fop)
 {
@ -979,8 +986,16 @@ ArrayBufferObject::releaseData(FreeOp* fop)
      case WASM:
        WasmArrayRawBuffer::Release(dataPointer());
        break;
-      case KIND_MASK:
-        MOZ_CRASH("bad bufferKind()");
+      case EXTERNAL:
+        if (refcountInfo()->unref) {
+            // The analyzer can't know for sure whether the embedder-supplied
+            // unref function will GC. We give the analyzer a hint here.
+            // (Doing a GC in the unref function is considered a programmer
+            // error.)
+            JS::AutoSuppressGCAnalysis nogc;
+            refcountInfo()->unref(dataPointer(), refcountInfo()->refUserData);
+        }
+        break;
    }
 }

@ -990,6 +1005,18 @@ ArrayBufferObject::setDataPointer(BufferContents contents, OwnsState ownsData)
    setSlot(DATA_SLOT, PrivateValue(contents.data()));
    setOwnsData(ownsData);
    setFlags((flags() & ~KIND_MASK) | contents.kind());
+
+    if (isExternal()) {
+        auto info = refcountInfo();
+        info->ref = contents.refFunc();
+        info->unref = contents.unrefFunc();
+        info->refUserData = contents.refUserData();
+        if (info->ref) {
+            // See comment in releaseData() for the explanation for this.
+            JS::AutoSuppressGCAnalysis nogc;
+            info->ref(dataPointer(), info->refUserData);
+        }
+    }
 }

 uint32_t
@ -1158,13 +1185,23 @@ ArrayBufferObject::create(JSContext* cx, uint32_t nbytes, BufferContents content
    bool allocated = false;
    if (contents) {
        if (ownsState == OwnsData) {
-            // The ABO is taking ownership, so account the bytes against the zone.
-            size_t nAllocated = nbytes;
-            if (contents.kind() == MAPPED)
-                nAllocated = JS_ROUNDUP(nbytes, js::gc::SystemPageSize());
-            else if (contents.kind() == WASM)
-                nAllocated = contents.wasmBuffer()->allocatedBytes();
-            cx->updateMallocCounter(nAllocated);
+            if (contents.kind() == EXTERNAL) {
+                // Store the RefcountInfo in the inline data slots so that we
+                // don't use up slots for it in non-refcounted array buffers.
+                size_t refcountInfoSlots = JS_HOWMANY(sizeof(RefcountInfo), sizeof(Value));
+                MOZ_ASSERT(reservedSlots + refcountInfoSlots <= NativeObject::MAX_FIXED_SLOTS,
+                           "RefcountInfo must fit in inline slots");
+                nslots += refcountInfoSlots;
+            } else {
+                // The ABO is taking ownership, so account the bytes against
+                // the zone.
+                size_t nAllocated = nbytes;
+                if (contents.kind() == MAPPED)
+                    nAllocated = JS_ROUNDUP(nbytes, js::gc::SystemPageSize());
+                else if (contents.kind() == WASM)
+                    nAllocated = contents.wasmBuffer()->allocatedBytes();
+                cx->updateMallocCounter(nAllocated);
+            }
        }
    } else {
        MOZ_ASSERT(ownsState == OwnsData);
@ -1263,7 +1300,7 @@ ArrayBufferObject::externalizeContents(JSContext* cx, Handle<ArrayBufferObject*>
    MOZ_ASSERT(!buffer->isDetached(), "must have contents to externalize");
    MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents());

-    BufferContents contents(buffer->dataPointer(), buffer->bufferKind());
+    BufferContents contents = buffer->contents();

    if (hasStealableContents) {
        buffer->setOwnsData(DoesntOwnData);
@ -1291,7 +1328,7 @@ ArrayBufferObject::stealContents(JSContext* cx, Handle<ArrayBufferObject*> buffe
                                        (buffer->isWasm() && !buffer->isPreparedForAsmJS()));
    assertSameCompartment(cx, buffer);

-    BufferContents oldContents(buffer->dataPointer(), buffer->bufferKind());
+    BufferContents oldContents = buffer->contents();

    if (hasStealableContents) {
        // Return the old contents and reset the detached buffer's data
@ -1803,12 +1840,30 @@ JS_NewArrayBufferWithContents(JSContext* cx, size_t nbytes, void* data)
    AssertHeapIsIdle();
    CHECK_REQUEST(cx);
    MOZ_ASSERT_IF(!data, nbytes == 0);
+
    ArrayBufferObject::BufferContents contents =
        ArrayBufferObject::BufferContents::create<ArrayBufferObject::PLAIN>(data);
    return ArrayBufferObject::create(cx, nbytes, contents, ArrayBufferObject::OwnsData,
                                     /* proto = */ nullptr, TenuredObject);
 }

+JS_PUBLIC_API(JSObject*)
+JS_NewExternalArrayBuffer(JSContext* cx, size_t nbytes, void* data,
+                          JS::BufferContentsRefFunc ref, JS::BufferContentsRefFunc unref,
+                          void* refUserData)
+{
+    AssertHeapIsIdle();
+    CHECK_REQUEST(cx);
+
+    MOZ_ASSERT(data);
+    MOZ_ASSERT(nbytes > 0);
+
+    ArrayBufferObject::BufferContents contents =
+        ArrayBufferObject::BufferContents::createExternal(data, ref, unref, refUserData);
+    return ArrayBufferObject::create(cx, nbytes, contents, ArrayBufferObject::OwnsData,
+                                     /* proto = */ nullptr, TenuredObject);
+}
+
 JS_PUBLIC_API(JSObject*)
 JS_NewArrayBufferWithExternalContents(JSContext* cx, size_t nbytes, void* data)
 {
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@ -180,6 +180,7 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
        PLAIN               = 0, // malloced or inline data
        WASM                = 1,
        MAPPED              = 2,
+        EXTERNAL            = 3,

        KIND_MASK           = 0x3
    };
@ -193,9 +194,9 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
        DETACHED            = 0x4,

        // The dataPointer() is owned by this buffer and should be released
-        // when no longer in use. Releasing the pointer may be done by either
-        // freeing or unmapping it, and how to do this is determined by the
-        // buffer's other flags.
+        // when no longer in use. Releasing the pointer may be done by freeing,
+        // invoking a dereference callback function, or unmapping, as
+        // determined by the buffer's other flags.
        //
        // Array buffers which do not own their data include buffers that
        // allocate their data inline, and buffers that are created lazily for
@ -225,11 +226,22 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
    class BufferContents {
        uint8_t* data_;
        BufferKind kind_;
+        JS::BufferContentsRefFunc ref_;
+        JS::BufferContentsRefFunc unref_;
+        void* refUserData_;

        friend class ArrayBufferObject;

-        BufferContents(uint8_t* data, BufferKind kind) : data_(data), kind_(kind) {
+        BufferContents(uint8_t* data, BufferKind kind, JS::BufferContentsRefFunc ref = nullptr,
+                       JS::BufferContentsRefFunc unref = nullptr, void* refUserData = nullptr)
+        : data_(data), kind_(kind), ref_(ref), unref_(unref), refUserData_(refUserData)
+        {
            MOZ_ASSERT((kind_ & ~KIND_MASK) == 0);
+            MOZ_ASSERT_IF(ref_ || unref_ || refUserData_, kind_ == EXTERNAL);
+
+            // BufferContents does not ref or unref the data since it is
+            // internal and short-lived. It is the caller's responsibility to
+            // ensure that the BufferContents does not outlive the data.
        }

      public:
@ -245,8 +257,18 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
            return BufferContents(static_cast<uint8_t*>(data), PLAIN);
        }

+        static BufferContents createExternal(void *data, JS::BufferContentsRefFunc ref,
+                                             JS::BufferContentsRefFunc unref,
+                                             void* refUserData = nullptr)
+        {
+            return BufferContents(static_cast<uint8_t*>(data), EXTERNAL, ref, unref, refUserData);
+        }
+
        uint8_t* data() const { return data_; }
        BufferKind kind() const { return kind_; }
+        JS::BufferContentsRefFunc refFunc() const { return ref_; }
+        JS::BufferContentsRefFunc unrefFunc() const { return unref_; }
+        void* refUserData() const { return refUserData_; }

        explicit operator bool() const { return data_ != nullptr; }
        WasmArrayRawBuffer* wasmBuffer() const;
@ -329,12 +351,23 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared

    uint8_t* inlineDataPointer() const;

+    struct RefcountInfo {
+        JS::BufferContentsRefFunc ref;
+        JS::BufferContentsRefFunc unref;
+        void* refUserData;
+    };
+    RefcountInfo* refcountInfo() const;
+
  public:
    uint8_t* dataPointer() const;
    SharedMem<uint8_t*> dataPointerShared() const;
    uint32_t byteLength() const;

    BufferContents contents() const {
+        if (isExternal()) {
+            return BufferContents(dataPointer(), EXTERNAL, refcountInfo()->ref,
+                                  refcountInfo()->unref, refcountInfo()->refUserData);
+        }
        return BufferContents(dataPointer(), bufferKind());
    }
    bool hasInlineData() const {
@ -355,6 +388,7 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared
    bool isPlain() const { return bufferKind() == PLAIN; }
    bool isWasm() const { return bufferKind() == WASM; }
    bool isMapped() const { return bufferKind() == MAPPED; }
+    bool isExternal() const { return bufferKind() == EXTERNAL; }
    bool isDetached() const { return flags() & DETACHED; }
    bool isPreparedForAsmJS() const { return flags() & FOR_ASMJS; }

--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@ -1112,6 +1112,12 @@ JSStructuredCloneWriter::parseTransferable()
        if (tObj->is<WasmMemoryObject>() && tObj->as<WasmMemoryObject>().isShared())
            return reportDataCloneError(JS_SCERR_SHMEM_TRANSFERABLE);

+        // External array buffers may be able to be transferred in the future,
+        // but that is not currently implemented.
+
+        if (tObj->is<ArrayBufferObject>() && tObj->as<ArrayBufferObject>().isExternal())
+            return reportDataCloneError(JS_SCERR_TRANSFERABLE);
+
        // No duplicates allowed
        auto p = transferableObjects.lookupForAdd(tObj);
        if (p)