From f085eb761cd7c595c7e5b29353cbe5d02de62261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Thu, 6 Jul 2023 11:41:18 +0000 Subject: [PATCH] Bug 1841314 - Part 6: Add marker to NewArrayBufferWithContents. r=sfink Differential Revision: https://phabricator.services.mozilla.com/D182644 --- dom/file/FileReader.cpp | 6 +++++- dom/xhr/XMLHttpRequestMainThread.cpp | 6 +++++- js/public/ArrayBuffer.h | 11 ++++++++--- js/src/vm/ArrayBufferObject.cpp | 9 +++++---- js/src/vm/StructuredClone.cpp | 6 +++++- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/dom/file/FileReader.cpp b/dom/file/FileReader.cpp index 93948671df19..12c1853ab121 100644 --- a/dom/file/FileReader.cpp +++ b/dom/file/FileReader.cpp @@ -201,7 +201,11 @@ void FileReader::OnLoadEndArrayBuffer() { JSContext* cx = jsapi.cx(); - mResultArrayBuffer = JS::NewArrayBufferWithContents(cx, mDataLen, mFileData); + // |mFileData| will be deallocated in FileReader's destructor when this + // ArrayBuffer allocation failed. + mResultArrayBuffer = JS::NewArrayBufferWithContents( + cx, mDataLen, mFileData, + JS::NewArrayBufferOutOfMemory::CallerMustFreeMemory); if (mResultArrayBuffer) { mFileData = nullptr; // Transfer ownership FreeDataAndDispatchSuccess(); diff --git a/dom/xhr/XMLHttpRequestMainThread.cpp b/dom/xhr/XMLHttpRequestMainThread.cpp index 23161957d341..2756aa186e5b 100644 --- a/dom/xhr/XMLHttpRequestMainThread.cpp +++ b/dom/xhr/XMLHttpRequestMainThread.cpp @@ -3927,7 +3927,11 @@ JSObject* ArrayBufferBuilder::TakeArrayBuffer(JSContext* aCx) { } } - JSObject* obj = JS::NewArrayBufferWithContents(aCx, mLength, mDataPtr); + // |mDataPtr| will be deallocated in ArrayBufferBuilder's destructor when this + // ArrayBuffer allocation failed. + JSObject* obj = JS::NewArrayBufferWithContents( + aCx, mLength, mDataPtr, + JS::NewArrayBufferOutOfMemory::CallerMustFreeMemory); if (!obj) { return nullptr; } diff --git a/js/public/ArrayBuffer.h b/js/public/ArrayBuffer.h index 284b7650264d..1b78ab4448ee 100644 --- a/js/public/ArrayBuffer.h +++ b/js/public/ArrayBuffer.h @@ -46,6 +46,12 @@ extern JS_PUBLIC_API JSObject* NewArrayBufferWithContents( JSContext* cx, size_t nbytes, mozilla::UniquePtr contents); +/** + * Marker enum to notify callers that the buffer contents must be freed manually + * when the ArrayBuffer allocation failed. + */ +enum class NewArrayBufferOutOfMemory { CallerMustFreeMemory }; + /** * Create a new ArrayBuffer with the given |contents|, which may be null only * if |nbytes == 0|. |contents| must be allocated compatible with deallocation @@ -61,9 +67,8 @@ extern JS_PUBLIC_API JSObject* NewArrayBufferWithContents( * call to this function, it could move those contents to a different location * and invalidate the provided pointer. */ -extern JS_PUBLIC_API JSObject* NewArrayBufferWithContents(JSContext* cx, - size_t nbytes, - void* contents); +extern JS_PUBLIC_API JSObject* NewArrayBufferWithContents( + JSContext* cx, size_t nbytes, void* contents, NewArrayBufferOutOfMemory); /** * Create a new ArrayBuffer, whose bytes are set to the values of the bytes in diff --git a/js/src/vm/ArrayBufferObject.cpp b/js/src/vm/ArrayBufferObject.cpp index 5a895c0ca52d..6e21124fe44d 100644 --- a/js/src/vm/ArrayBufferObject.cpp +++ b/js/src/vm/ArrayBufferObject.cpp @@ -1927,7 +1927,9 @@ JS_PUBLIC_API JSObject* JS::NewArrayBuffer(JSContext* cx, size_t nbytes) { JS_PUBLIC_API JSObject* JS::NewArrayBufferWithContents( JSContext* cx, size_t nbytes, mozilla::UniquePtr contents) { - auto* result = NewArrayBufferWithContents(cx, nbytes, contents.get()); + auto* result = NewArrayBufferWithContents( + cx, nbytes, contents.get(), + JS::NewArrayBufferOutOfMemory::CallerMustFreeMemory); if (result) { // If and only if an ArrayBuffer is successfully created, ownership of // |contents| is transferred to the new ArrayBuffer. @@ -1936,9 +1938,8 @@ JS_PUBLIC_API JSObject* JS::NewArrayBufferWithContents( return result; } -JS_PUBLIC_API JSObject* JS::NewArrayBufferWithContents(JSContext* cx, - size_t nbytes, - void* data) { +JS_PUBLIC_API JSObject* JS::NewArrayBufferWithContents( + JSContext* cx, size_t nbytes, void* data, NewArrayBufferOutOfMemory) { AssertHeapIsIdle(); CHECK_THREAD(cx); MOZ_ASSERT_IF(!data, nbytes == 0); diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index 7eafc89113a5..b7a6cd250d10 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -3261,7 +3261,11 @@ bool JSStructuredCloneReader::readTransferMap() { MOZ_ASSERT(data == JS::SCTAG_TMO_ALLOC_DATA || data == JS::SCTAG_TMO_MAPPED_DATA); if (data == JS::SCTAG_TMO_ALLOC_DATA) { - obj = JS::NewArrayBufferWithContents(cx, nbytes, content); + // When the ArrayBuffer can't be allocated, |content| will be free'ed + // in `JSStructuredCloneData::discardTransferables()`. + obj = JS::NewArrayBufferWithContents( + cx, nbytes, content, + JS::NewArrayBufferOutOfMemory::CallerMustFreeMemory); } else if (data == JS::SCTAG_TMO_MAPPED_DATA) { obj = JS::NewMappedArrayBufferWithContents(cx, nbytes, content); }