Bug 1564821 - Response must call JS::ReadableStreamReleaseCCObject() if ::GetBody() fails, r=smaug, a=abillings
authorAndrea Marchesini <amarchesini@mozilla.com>
Fri, 12 Jul 2019 23:43:16 +0000
changeset 544554 b0cc801fe229051418ccfba5c5ed9b8f5bba710c
parent 544553 593102394614ba082947312b6743118b4c6df09f
child 544555 e5d98eda2ec359a0968c567076b1a625cb6c99ce
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, abillings
bugs1564821
milestone69.0
Bug 1564821 - Response must call JS::ReadableStreamReleaseCCObject() if ::GetBody() fails, r=smaug, a=abillings Differential Revision: https://phabricator.services.mozilla.com/D37711
dom/base/BodyStream.h
dom/fetch/Fetch.cpp
--- a/dom/base/BodyStream.h
+++ b/dom/base/BodyStream.h
@@ -42,16 +42,26 @@ class BodyStream final : public nsIInput
                          public nsIObserver,
                          public nsSupportsWeakReference,
                          private JS::ReadableStreamUnderlyingSource {
  public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIINPUTSTREAMCALLBACK
   NS_DECL_NSIOBSERVER
 
+  // This method creates a JS ReadableStream object and it makes a CC cycle
+  // between it and the aStreamHolder.
+  // The aStreamHolder must do the following operations after calling this
+  // method:
+  // - it must store the JS ReadableStream and returns it in
+  //   BodyStreamHolder::GetReadableStreamBody().
+  // - it must trace the JS ReadableStream.
+  // - if the operation has to be aborted, or the stream has to be released for
+  //   any reason, the aStreamHolder must call
+  //   JS::ReadableStreamReleaseCCObject().
   static void Create(JSContext* aCx, BodyStreamHolder* aStreamHolder,
                      nsIGlobalObject* aGlobal, nsIInputStream* aInputStream,
                      JS::MutableHandle<JSObject*> aStream, ErrorResult& aRv);
 
   void Close();
 
   static nsresult RetrieveInputStream(
       JS::ReadableStreamUnderlyingSource* aUnderlyingReadableStreamSource,
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -1323,16 +1323,21 @@ void FetchBody<Derived>::GetBody(JSConte
   BodyStream::Create(aCx, this, DerivedClass()->GetParentObject(), inputStream,
                      &body, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
   MOZ_ASSERT(body);
 
+  // We must break the cycle between the body and the stream in case we do an
+  // early return.
+  auto raii =
+      mozilla::MakeScopeExit([&] { JS::ReadableStreamReleaseCCObject(body); });
+
   // If the body has been already consumed, we lock the stream.
   bool bodyUsed = GetBodyUsed(aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
   if (bodyUsed) {
     LockStream(aCx, body, aRv);
     if (NS_WARN_IF(aRv.Failed())) {
@@ -1347,16 +1352,18 @@ void FetchBody<Derived>::GetBody(JSConte
       if (NS_WARN_IF(aRv.Failed())) {
         return;
       }
     } else if (!IsFollowing()) {
       Follow(signalImpl);
     }
   }
 
+  raii.release();
+
   mReadableStreamBody = body;
   aBodyOut.set(mReadableStreamBody);
 }
 
 template void FetchBody<Request>::GetBody(JSContext* aCx,
                                           JS::MutableHandle<JSObject*> aMessage,
                                           ErrorResult& aRv);