Bug 1941210 - Strongly grab FetchStreamReader while waiting for writing r=smaug

Nothing strongly grabs ReadableStream nor FetchStreamReader while waiting for nsIAsyncOutputStream to respond. mAsyncWaitReader should now strongly grab the reader until the output stream responds.

Differential Revision: https://phabricator.services.mozilla.com/D234031

dom/fetch/FetchStreamReader.cpp

@@ -72,9 +72,17 @@ nsresult OutputStreamHolder::AsyncWait(uint32_t aFlags,
                                        uint32_t aRequestedCount,
                                        nsIEventTarget* aEventTarget) {
   mAsyncWaitWorkerRef = mWorkerRef;
+  // Grab the strong reference for the reader but only when we are waiting for
+  // the output stream, because it means we still have things to write.
+  // (WAIT_CLOSURE_ONLY happens when waiting for ReadableStream to respond, at
+  // which point the pull callback should get an indirect strong reference via
+  // the controller argument.)
+  mAsyncWaitReader =
+      aFlags == nsIAsyncOutputStream::WAIT_CLOSURE_ONLY ? nullptr : mReader;
   nsresult rv = mOutput->AsyncWait(this, aFlags, aRequestedCount, aEventTarget);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     mAsyncWaitWorkerRef = nullptr;
+    mAsyncWaitReader = nullptr;
   }
   return rv;
 }
@@ -84,10 +92,16 @@ NS_IMETHODIMP OutputStreamHolder::OnOutputStreamReady(
   // We may get called back after ::Shutdown()
   if (!mReader) {
     mAsyncWaitWorkerRef = nullptr;
+    MOZ_ASSERT(!mAsyncWaitReader);
     return NS_OK;
   }
-  if (!mReader->OnOutputStreamReady()) {
+
+  // mAsyncWaitReader may be reset during OnOutputStreamReady, make sure to let
+  // it live during the call
+  RefPtr<FetchStreamReader> reader = mReader.get();
+  if (!reader->OnOutputStreamReady()) {
     mAsyncWaitWorkerRef = nullptr;
+    mAsyncWaitReader = nullptr;
     return NS_OK;
   }
   return NS_OK;

dom/fetch/FetchStreamReader.h

@@ -51,6 +51,7 @@ class OutputStreamHolder final : public nsIOutputStreamCallback {
  private:
   ~OutputStreamHolder();
 
+  RefPtr<FetchStreamReader> mAsyncWaitReader;
   // WeakPtr to avoid cycles
   WeakPtr<FetchStreamReader> mReader;
   // To ensure the worker sticks around

testing/web-platform/tests/fetch/api/basic/gc.any.js

@@ -0,0 +1,19 @@
+// META: global=window,worker
+// META: script=/common/gc.js
+
+promise_test(async () => {
+  let i = 0;
+  const repeat = 5;
+  const buffer = await new Response(new ReadableStream({
+    pull(c) {
+      if (i >= repeat) {
+        c.close();
+        return;
+      }
+      ++i;
+      c.enqueue(new Uint8Array([0]))
+      garbageCollect();
+    }
+  })).arrayBuffer();
+  assert_equals(buffer.byteLength, repeat, `The buffer should be ${repeat}-byte long`);
+}, "GC/CC should not abruptly close the stream while being consumed by Response");