Bug 1754004 - Part 18: Ensure AsyncWait callbacks are cleared when NS_AsyncCopy completes, r=xpcom-reviewers,mccr8

When the NS_AsyncCopy completes, there may still be outstanding
AsyncWait callbacks which have not been invoked yet, due to two
AsyncWait callbacks being registered each time Process() yields (one to
wait for the blocked stream, and the other with WAIT_CLOSURE_ONLY to
handle errors), and only one callback being needed to resume processing.

This change ensures that any outstanding AsyncWait callbacks are
cancelled, breaking references from the streams to the
nsAStreamCallback. Some streams (such as nsPipe and DataPipe) may also
leak if there are oustanding AsyncWait callbacks, due to the need to
keep the stream alive so it can be passed into the callback, which this
helps avoid.

Previously leaks were largely avoided due to the call to `Close()`
cancelling the callbacks for us, however not all callsites specify to
close the source/sink.

This should also help avoid an unnecessary dispatch after the copy
completes due to our AsyncWait callback being invoked when the
source/sink stream is closed.

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

netwerk/test/gtest/TestInputStreamTransport.cpp

@@ -154,6 +154,10 @@ class BlockingAsyncStream final : public nsIAsyncInputStream {
   NS_IMETHOD
   AsyncWait(nsIInputStreamCallback* aCallback, uint32_t aFlags,
             uint32_t aRequestedCount, nsIEventTarget* aEventTarget) override {
+    if (!aCallback) {
+      return NS_OK;
+    }
+
     RefPtr<BlockingAsyncStream> self = this;
     nsCOMPtr<nsIInputStreamCallback> callback = aCallback;
 

xpcom/io/nsStreamUtils.cpp

@@ -318,41 +318,52 @@ class nsAStreamCopier : public nsIInputStreamCallback,
           // more source data, be sure to observe failures on output end.
           mAsyncSource->AsyncWait(this, 0, 0, nullptr);
 
-          if (mAsyncSink)
+          if (mAsyncSink) {
             mAsyncSink->AsyncWait(this, nsIAsyncOutputStream::WAIT_CLOSURE_ONLY,
                                   0, nullptr);
+          }
           break;
-        } else if (sinkCondition == NS_BASE_STREAM_WOULD_BLOCK && mAsyncSink) {
+        }
+        if (sinkCondition == NS_BASE_STREAM_WOULD_BLOCK && mAsyncSink) {
           // need to wait for more room in the sink.  while waiting for
           // more room in the sink, be sure to observer failures on the
           // input end.
           mAsyncSink->AsyncWait(this, 0, 0, nullptr);
 
-          if (mAsyncSource)
+          if (mAsyncSource) {
             mAsyncSource->AsyncWait(
                 this, nsIAsyncInputStream::WAIT_CLOSURE_ONLY, 0, nullptr);
+          }
           break;
         }
       }
       if (copyFailed || canceled) {
+        if (mAsyncSource) {
+          // cancel any previously-registered AsyncWait callbacks to avoid leaks
+          mAsyncSource->AsyncWait(nullptr, 0, 0, nullptr);
+        }
         if (mCloseSource) {
           // close source
-          if (mAsyncSource)
+          if (mAsyncSource) {
             mAsyncSource->CloseWithStatus(canceled ? cancelStatus
                                                    : sinkCondition);
-          else {
+          } else {
             mSource->Close();
           }
         }
         mAsyncSource = nullptr;
         mSource = nullptr;
 
+        if (mAsyncSink) {
+          // cancel any previously-registered AsyncWait callbacks to avoid leaks
+          mAsyncSink->AsyncWait(nullptr, 0, 0, nullptr);
+        }
         if (mCloseSink) {
           // close sink
-          if (mAsyncSink)
+          if (mAsyncSink) {
             mAsyncSink->CloseWithStatus(canceled ? cancelStatus
                                                  : sourceCondition);
-          else {
+          } else {
             // If we have an nsISafeOutputStream, and our
             // sourceCondition and sinkCondition are not set to a
             // failure state, finish writing.