Bug 995278 - JS_NewArrayBufferContents frees user data on error. r=sfink
authorAnuj Agarwal <anujagarwal464@gmail.com>
Sun, 27 Apr 2014 01:22:00 -0400
changeset 200053 ebdd57580809e75dc1def2721253883c4ed63ed9
parent 200052 5aac645646304195a0edb048461303b7b82ac107
child 200054 eaa0d64e544e0dd8ba29ed2ec66e8ecdb5ab248e
push id486
push userasasaki@mozilla.com
push dateMon, 14 Jul 2014 18:39:42 +0000
treeherdermozilla-release@d33428174ff1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs995278
milestone31.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 995278 - JS_NewArrayBufferContents frees user data on error. r=sfink
content/base/src/nsXMLHttpRequest.cpp
js/src/jsapi-tests/testMappedArrayBuffer.cpp
js/src/jsapi.h
js/src/vm/ArrayBufferObject.cpp
js/src/vm/StructuredClone.cpp
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -3925,23 +3925,22 @@ ArrayBufferBuilder::getArrayBuffer(JSCon
   // added
   if (mCapacity > mLength || mLength == 0) {
     if (!setCapacity(mLength)) {
       return nullptr;
     }
   }
 
   JSObject* obj = JS_NewArrayBufferWithContents(aCx, mLength, mDataPtr);
+  mDataPtr = nullptr;
+  mLength = mCapacity = 0;
   if (!obj) {
+    js_free(mDataPtr);
     return nullptr;
   }
-
-  mDataPtr = nullptr;
-  mLength = mCapacity = 0;
-
   return obj;
 }
 
 /* static */ bool
 ArrayBufferBuilder::areOverlappingRegions(const uint8_t* aStart1,
                                           uint32_t aLength1,
                                           const uint8_t* aStart2,
                                           uint32_t aLength2)
--- a/js/src/jsapi-tests/testMappedArrayBuffer.cpp
+++ b/js/src/jsapi-tests/testMappedArrayBuffer.cpp
@@ -60,17 +60,20 @@ BEGIN_TEST(testMappedArrayBuffer_bug9451
 JSObject *CreateNewObject(const int offset, const int length)
 {
     int fd = open(test_filename, O_RDONLY);
     void *ptr = JS_CreateMappedArrayBufferContents(fd, offset, length);
     close(fd);
     if (!ptr)
         return nullptr;
     JSObject *obj = JS_NewMappedArrayBufferWithContents(cx, length, ptr);
-
+    if (!obj) {
+        JS_ReleaseMappedArrayBufferContents(ptr, length);
+        return nullptr;
+    }
     return obj;
 }
 
 bool VerifyObject(JS::HandleObject obj, const int offset, const int length, const bool mapped)
 {
     CHECK(obj);
     CHECK(JS_IsArrayBufferObject(obj));
     CHECK_EQUAL(JS_GetArrayBufferByteLength(obj), length);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3166,19 +3166,18 @@ JS_ClearNonGlobalObject(JSContext *cx, J
 /*
  * Assign 'undefined' to all of the object's non-reserved slots. Note: this is
  * done for all slots, regardless of the associated property descriptor.
  */
 JS_PUBLIC_API(void)
 JS_SetAllNonReservedSlotsToUndefined(JSContext *cx, JSObject *objArg);
 
 /*
- * Create a new array buffer with the given contents. The new array buffer
- * takes ownership: after calling this function, do not free |contents| or use
- * |contents| from another thread.
+ * Create a new array buffer with the given contents. On success, the ownership
+ * is transferred to the new array buffer.
  */
 extern JS_PUBLIC_API(JSObject *)
 JS_NewArrayBufferWithContents(JSContext *cx, size_t nbytes, void *contents);
 
 /*
  * Steal the contents of the given array buffer. The array buffer has its
  * 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
@@ -3200,17 +3199,18 @@ JS_AllocateArrayBufferContents(JSContext
  * Reallocate memory allocated by JS_AllocateArrayBufferContents, growing or
  * shrinking it as appropriate. If oldContents is null then this behaves like
  * JS_AllocateArrayBufferContents.
  */
 extern JS_PUBLIC_API(void *)
 JS_ReallocateArrayBufferContents(JSContext *cx, uint32_t nbytes, void *oldContents, uint32_t oldNbytes);
 
 /*
- * Create a new mapped array buffer with the given memory mapped contents.
+ * Create a new mapped array buffer with the given memory mapped contents. On success,
+ * the ownership is transferred to the new mapped array buffer.
  */
 extern JS_PUBLIC_API(JSObject *)
 JS_NewMappedArrayBufferWithContents(JSContext *cx, size_t nbytes, void *contents);
 
 /*
  * Create memory mapped array buffer contents.
  * Caller must take care of closing fd after calling this function.
  */
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -646,21 +646,19 @@ ArrayBufferObject::create(JSContext *cx,
                 return nullptr;
         }
     }
 
     JS_ASSERT(!(class_.flags & JSCLASS_HAS_PRIVATE));
     gc::AllocKind allocKind = GetGCObjectKind(nslots);
 
     Rooted<ArrayBufferObject*> obj(cx, NewBuiltinClassInstance<ArrayBufferObject>(cx, allocKind, newKind));
-    if (!obj) {
-        if (data)
-            js_free(data);
+    if (!obj)
         return nullptr;
-    }
+
     JS_ASSERT(obj->getClass() == &class_);
 
     JS_ASSERT(!gc::IsInsideNursery(cx->runtime(), obj));
 
     if (data) {
         obj->initialize(nbytes, data, OwnsData);
         if (mapped)
             obj->setIsMappedArrayBuffer();
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -1567,17 +1567,19 @@ JSStructuredCloneReader::readTransferMap
                 ReportErrorTransferable(cx, callbacks);
                 return false;
             }
             if (!callbacks->readTransfer(cx, this, tag, content, extraData, closure, &obj))
                 return false;
             MOZ_ASSERT(obj);
             MOZ_ASSERT(!cx->isExceptionPending());
         }
-
+        
+        // On failure, the buffer will still own the data (since its ownership will not get set to SCTAG_TMO_UNOWNED),
+        // so the data will be freed by ClearStructuredClone
         if (!obj)
             return false;
 
         // Mark the SCTAG_TRANSFER_MAP_* entry as no longer owned by the input
         // buffer.
         *pos = PairToUInt64(tag, JS::SCTAG_TMO_UNOWNED);
         MOZ_ASSERT(headerPos < pos && pos < in.end());