From 5dd1b01d137d1663d934d205867debd15ec2324b Mon Sep 17 00:00:00 2001 From: Philip Chimento Date: Thu, 22 Feb 2018 21:03:00 -0800 Subject: [PATCH] Bug 1430438 - Allow reference counted data in JS_NewArrayBufferWithContents(). r=sfink --HG-- extra : rebase_source : fbea1f2f9408247b2375c575ca2d8997d43d7200 --- js/src/jsapi-tests/testArrayBuffer.cpp | 115 +++++++++++++++++++++ js/src/jsapi-tests/testStructuredClone.cpp | 56 ++++++++++ js/src/jsapi-tests/tests.h | 37 +++++++ js/src/jsapi.h | 36 +++++++ js/src/vm/ArrayBufferObject.cpp | 77 ++++++++++++-- js/src/vm/ArrayBufferObject.h | 42 +++++++- js/src/vm/StructuredClone.cpp | 6 ++ 7 files changed, 354 insertions(+), 15 deletions(-) diff --git a/js/src/jsapi-tests/testArrayBuffer.cpp b/js/src/jsapi-tests/testArrayBuffer.cpp index ed00f09755a6..538cde665939 100644 --- 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(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(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(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(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) \ No newline at end of file diff --git a/js/src/jsapi-tests/testStructuredClone.cpp b/js/src/jsapi-tests/testStructuredClone.cpp index bfc7fda5a285..8433712b5bb1 100644 --- 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(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; diff --git a/js/src/jsapi-tests/tests.h b/js/src/jsapi-tests/tests.h index a51592d86587..f1db7f4ce176 100644 --- 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(userData); + MOZ_ASSERT(self->contents() == contents); + self->incref(); + } + + static void decCallback(void* contents, void* userData) { + auto self = static_cast(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 diff --git a/js/src/jsapi.h b/js/src/jsapi.h index eff474602f99..d9d520f97446 100644 --- 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. diff --git a/js/src/vm/ArrayBufferObject.cpp b/js/src/vm/ArrayBufferObject.cpp index d0f06d9fd5d9..0a342a6850c7 100644 --- a/js/src/vm/ArrayBufferObject.cpp +++ b/js/src/vm/ArrayBufferObject.cpp @@ -964,6 +964,13 @@ ArrayBufferObject::dataPointerShared() const return SharedMem::unshared(getSlot(DATA_SLOT).toPrivate()); } +ArrayBufferObject::RefcountInfo* +ArrayBufferObject::refcountInfo() const +{ + MOZ_ASSERT(isExternal()); + return reinterpret_cast(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 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 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(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) { diff --git a/js/src/vm/ArrayBufferObject.h b/js/src/vm/ArrayBufferObject.h index fe29b9a8fe48..27960539e60a 100644 --- 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(data), PLAIN); } + static BufferContents createExternal(void *data, JS::BufferContentsRefFunc ref, + JS::BufferContentsRefFunc unref, + void* refUserData = nullptr) + { + return BufferContents(static_cast(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 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; } diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index 80af35b3761e..2c736c001452 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -1112,6 +1112,12 @@ JSStructuredCloneWriter::parseTransferable() if (tObj->is() && tObj->as().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() && tObj->as().isExternal()) + return reportDataCloneError(JS_SCERR_TRANSFERABLE); + // No duplicates allowed auto p = transferableObjects.lookupForAdd(tObj); if (p)