Bug 1557781 - Better life-time management for BodyStream/FetchStream - part 1 - JSCLASS_PRIVATE_IS_NSISUPPORTS, r=jonco,jorendorff
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 01 Jul 2019 19:59:33 +0000
changeset 540484 b43fac4ccd1dfd19f30d5d8ac74d0c622a9f1521
parent 540483 2db482852bec317c0a2d270f178821095f4e3163
child 540485 ea96413122a8329ccb00eacc31416354c854b7b0
push id11529
push userarchaeopteryx@coole-files.de
push dateThu, 04 Jul 2019 15:22:33 +0000
treeherdermozilla-beta@ebb510a784b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco, jorendorff
bugs1557781
milestone69.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 1557781 - Better life-time management for BodyStream/FetchStream - part 1 - JSCLASS_PRIVATE_IS_NSISUPPORTS, r=jonco,jorendorff Differential Revision: https://phabricator.services.mozilla.com/D35528
js/public/Stream.h
js/src/builtin/Stream.cpp
js/src/builtin/Stream.h
--- a/js/public/Stream.h
+++ b/js/public/Stream.h
@@ -161,29 +161,51 @@ class JS_PUBLIC_API ReadableStreamUnderl
  */
 extern JS_PUBLIC_API JSObject* NewReadableDefaultStreamObject(
     JSContext* cx, HandleObject underlyingSource = nullptr,
     HandleFunction size = nullptr, double highWaterMark = 1,
     HandleObject proto = nullptr);
 
 /**
  * Returns a new instance of the ReadableStream builtin class in the current
- * compartment. If a |proto| is passed, that gets set as the instance's
- * [[Prototype]] instead of the original value of |ReadableStream.prototype|.
+ * compartment.
+ *
+ * The instance is a byte stream backed by an embedding-provided underlying
+ * source, using the virtual methods of `underlyingSource` as callbacks. The
+ * embedding must ensure that `*underlyingSource` lives as long as the new
+ * stream object. The JS engine will call the finalize() method when the stream
+ * object is destroyed.
+ *
+ * `nsISupportsObject_alreadyAddreffed` is an optional pointer that can be used
+ * to make the new stream participate in Gecko's cycle collection. Here are the
+ * rules for using this parameter properly:
+ *
+ * -   `*underlyingSource` must not be a cycle-collected object. (It would lead
+ *     to memory leaks as the cycle collector would not be able to collect
+ *     cycles containing that object.)
  *
- * The instance is optimized for operating as a byte stream backed by an
- * embedding-provided underlying source, using the virtual methods of
- * |underlyingSource| as callbacks.
+ * -   `*underlyingSource` must not contain nsCOMPtrs that point to cycle-
+ *     collected objects. (Same reason.)
+ *
+ * -   `*underlyingSource` may contain a pointer to a single cycle-collected
+ *     object.
+ *
+ * -   The pointer may be stored in `*underlyingSource` as a raw pointer.
  *
- * Note: The embedding must ensure that |*underlyingSource| lives as long as
- * the new stream object. The JS engine will call the finalize() method when
- * the stream object is destroyed.
+ * -   The pointer to the nsISupports interface of the same object must be
+ *     passed as the `nsISupportsObject_alreadyAddreffed` parameter to this
+ *     function. (This is how the cycle collector knows about it, so omitting
+ *     this would again cause leaks.)
+ *
+ * If `proto` is non-null, it is used as the instance's [[Prototype]] instead
+ * of the original value of `ReadableStream.prototype`.
  */
 extern JS_PUBLIC_API JSObject* NewReadableExternalSourceStreamObject(
     JSContext* cx, ReadableStreamUnderlyingSource* underlyingSource,
+    void* nsISupportsObject_alreadyAddreffed = nullptr,
     HandleObject proto = nullptr);
 
 /**
  * Returns the embedding-provided underlying source of the given |stream|.
  *
  * Can be used to optimize operations if both the underlying source and the
  * intended sink are embedding-provided. In that case it might be
  * preferrable to pipe data directly from source to sink without interacting
--- a/js/src/builtin/Stream.cpp
+++ b/js/src/builtin/Stream.cpp
@@ -470,18 +470,20 @@ const Class TeeState::class_ = {"TeeStat
 /*** 3.2. Class ReadableStream **********************************************/
 
 static MOZ_MUST_USE bool SetUpExternalReadableByteStreamController(
     JSContext* cx, Handle<ReadableStream*> stream,
     JS::ReadableStreamUnderlyingSource* source);
 
 ReadableStream* ReadableStream::createExternalSourceStream(
     JSContext* cx, JS::ReadableStreamUnderlyingSource* source,
+    void* nsISupportsObject_alreadyAddreffed /* = nullptr */,
     HandleObject proto /* = nullptr */) {
-  Rooted<ReadableStream*> stream(cx, create(cx, proto));
+  Rooted<ReadableStream*> stream(
+      cx, create(cx, nsISupportsObject_alreadyAddreffed, proto));
   if (!stream) {
     return nullptr;
   }
 
   if (!SetUpExternalReadableByteStreamController(cx, stream, source)) {
     return nullptr;
   }
 
@@ -531,17 +533,18 @@ bool ReadableStream::constructor(JSConte
   // Implicit in the spec: Set this to
   //     OrdinaryCreateFromConstructor(NewTarget, ...).
   // Step 1: Perform ! InitializeReadableStream(this).
   RootedObject proto(cx);
   if (!GetPrototypeFromBuiltinConstructor(cx, args, JSProto_ReadableStream,
                                           &proto)) {
     return false;
   }
-  Rooted<ReadableStream*> stream(cx, ReadableStream::create(cx, proto));
+  Rooted<ReadableStream*> stream(cx,
+                                 ReadableStream::create(cx, nullptr, proto));
   if (!stream) {
     return false;
   }
 
   // Step 2: Let size be ? GetV(strategy, "size").
   RootedValue size(cx);
   if (!GetProperty(cx, strategy, cx->names().size, &size)) {
     return false;
@@ -795,17 +798,19 @@ static bool ReadableStream_tee(JSContext
 static const JSFunctionSpec ReadableStream_methods[] = {
     JS_FN("cancel", ReadableStream_cancel, 1, 0),
     JS_FN("getReader", ReadableStream_getReader, 0, 0),
     JS_FN("tee", ReadableStream_tee, 0, 0), JS_FS_END};
 
 static const JSPropertySpec ReadableStream_properties[] = {
     JS_PSG("locked", ReadableStream_locked, 0), JS_PS_END};
 
-CLASS_SPEC(ReadableStream, 0, SlotCount, 0, 0, JS_NULL_CLASS_OPS);
+CLASS_SPEC(ReadableStream, 0, SlotCount, 0,
+           JSCLASS_PRIVATE_IS_NSISUPPORTS | JSCLASS_HAS_PRIVATE,
+           JS_NULL_CLASS_OPS);
 
 /*** 3.3. General readable stream abstract operations ***********************/
 
 // Streams spec, 3.3.1. AcquireReadableStreamBYOBReader ( stream )
 // Always inlined.
 
 // Streams spec, 3.3.2. AcquireReadableStreamDefaultReader ( stream )
 // Always inlined. See CreateReadableStreamDefaultReader.
@@ -848,17 +853,18 @@ MOZ_MUST_USE ReadableStream* CreateReada
   // Step 2: If sizeAlgorithm was not passed, set it to an algorithm that
   //         returns 1 (implicit).
   // Step 3: Assert: ! IsNonNegativeNumber(highWaterMark) is true.
   MOZ_ASSERT(highWaterMark >= 0);
 
   // Step 4: Let stream be ObjectCreate(the original value of ReadableStream's
   //         prototype property).
   // Step 5: Perform ! InitializeReadableStream(stream).
-  Rooted<ReadableStream*> stream(cx, ReadableStream::create(cx, proto));
+  Rooted<ReadableStream*> stream(cx,
+                                 ReadableStream::create(cx, nullptr, proto));
   if (!stream) {
     return nullptr;
   }
 
   // Step 6: Let controller be ObjectCreate(the original value of
   //         ReadableStreamDefaultController's prototype property).
   // Step 7: Perform ? SetUpReadableStreamDefaultController(stream,
   //         controller, startAlgorithm, pullAlgorithm, cancelAlgorithm,
@@ -878,25 +884,29 @@ MOZ_MUST_USE ReadableStream* CreateReada
 //                          [, highWaterMark [, autoAllocateChunkSize ] ] )
 // Not implemented.
 
 /**
  * Streams spec, 3.3.5. InitializeReadableStream ( stream )
  */
 MOZ_MUST_USE /* static */
     ReadableStream*
-    ReadableStream::create(JSContext* cx, HandleObject proto /* = nullptr */) {
+    ReadableStream::create(
+        JSContext* cx, void* nsISupportsObject_alreadyAddreffed /* = nullptr */,
+        HandleObject proto /* = nullptr */) {
   // In the spec, InitializeReadableStream is always passed a newly created
   // ReadableStream object. We instead create it here and return it below.
   Rooted<ReadableStream*> stream(
       cx, NewObjectWithClassProto<ReadableStream>(cx, proto));
   if (!stream) {
     return nullptr;
   }
 
+  JS_SetPrivate(stream, nsISupportsObject_alreadyAddreffed);
+
   // Step 1: Set stream.[[state]] to "readable".
   stream->initStateBits(Readable);
   MOZ_ASSERT(stream->readable());
 
   // Step 2: Set stream.[[reader]] and stream.[[storedError]] to
   //         undefined (implicit).
   MOZ_ASSERT(!stream->hasReader());
   MOZ_ASSERT(stream->storedError().isUndefined());
@@ -4486,27 +4496,28 @@ JS_PUBLIC_API JSObject* JS::NewReadableD
     return nullptr;
   }
 
   return stream;
 }
 
 JS_PUBLIC_API JSObject* JS::NewReadableExternalSourceStreamObject(
     JSContext* cx, JS::ReadableStreamUnderlyingSource* underlyingSource,
+    void* nsISupportsObject_alreadyAddreffed /* = nullptr */,
     HandleObject proto /* = nullptr */) {
   MOZ_ASSERT(!cx->zone()->isAtomsZone());
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   MOZ_ASSERT(underlyingSource);
   MOZ_ASSERT((uintptr_t(underlyingSource) & 1) == 0,
              "external underlying source pointers must be aligned");
   cx->check(proto);
 
-  return ReadableStream::createExternalSourceStream(cx, underlyingSource,
-                                                    proto);
+  return ReadableStream::createExternalSourceStream(
+      cx, underlyingSource, nsISupportsObject_alreadyAddreffed, proto);
 }
 
 JS_PUBLIC_API bool JS::IsReadableStream(JSObject* obj) {
   return obj->canUnwrapAs<ReadableStream>();
 }
 
 JS_PUBLIC_API bool JS::IsReadableStreamReader(JSObject* obj) {
   return obj->canUnwrapAs<ReadableStreamDefaultReader>();
--- a/js/src/builtin/Stream.h
+++ b/js/src/builtin/Stream.h
@@ -100,20 +100,22 @@ class ReadableStream : public NativeObje
   void setStoredError(HandleValue value) {
     setFixedSlot(Slot_StoredError, value);
   }
 
   JS::ReadableStreamMode mode() const;
 
   bool locked() const;
 
-  static MOZ_MUST_USE ReadableStream* create(JSContext* cx,
-                                             HandleObject proto = nullptr);
+  static MOZ_MUST_USE ReadableStream* create(
+      JSContext* cx, void* nsISupportsObject_alreadyAddreffed = nullptr,
+      HandleObject proto = nullptr);
   static ReadableStream* createExternalSourceStream(
       JSContext* cx, JS::ReadableStreamUnderlyingSource* source,
+      void* nsISupportsObject_alreadyAddreffed = nullptr,
       HandleObject proto = nullptr);
 
   static bool constructor(JSContext* cx, unsigned argc, Value* vp);
   static const ClassSpec classSpec_;
   static const Class class_;
   static const ClassSpec protoClassSpec_;
   static const Class protoClass_;
 };