Bug 1754031 - AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
authorNika Layzell <nika@thelayzells.com>
Tue, 03 May 2022 23:30:30 +0000
changeset 616058 14a5f40fc11dc7fdf627338e33353c2a2c819609
parent 616057 9bfc934172e97f7f7a7f80bc0cfe569f4b2964a1
child 616059 d70810d7b63a12b670343084a301796fa2b7482b
push id162160
push usernlayzell@mozilla.com
push dateWed, 04 May 2022 00:03:58 +0000
treeherderautoland@63f17a06eba9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, necko-reviewers, dragana
bugs1754031
milestone102.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 1754031 - AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana This operation is often performed by nsAStreamCopier when switching between the source and sink streams, in order to enable or disable the WAIT_CLOSURE_ONLY flag. Failing to reset the wait flags can lead to a NS_AsyncCopy hanging until the source or sink are closed when is alternating between waiting on input and output streams. This patch relaxes the incorrect checks on various input streams. Differential Revision: https://phabricator.services.mozilla.com/D141034
dom/file/ipc/RemoteLazyInputStream.cpp
dom/file/uri/BlobURLInputStream.cpp
netwerk/base/PartiallySeekableInputStream.cpp
netwerk/base/nsBufferedStreams.cpp
netwerk/base/nsMIMEInputStream.cpp
netwerk/base/nsStreamTransportService.cpp
xpcom/io/InputStreamLengthWrapper.cpp
xpcom/io/SlicedInputStream.cpp
xpcom/io/nsMultiplexInputStream.cpp
--- a/dom/file/ipc/RemoteLazyInputStream.cpp
+++ b/dom/file/ipc/RemoteLazyInputStream.cpp
@@ -417,29 +417,31 @@ RemoteLazyInputStream::AsyncWait(nsIInpu
         mInputStreamCallbackEventTarget = aEventTarget;
         mState = ePending;
 
         mActor->StreamNeeded(this, aEventTarget);
         return NS_OK;
 
       // We are still waiting for the remote inputStream
       case ePending: {
-        if (mInputStreamCallback && aCallback) {
+        if (NS_WARN_IF(mInputStreamCallback && aCallback &&
+                       mInputStreamCallback != aCallback)) {
           return NS_ERROR_FAILURE;
         }
 
         mInputStreamCallback = aCallback;
         mInputStreamCallbackEventTarget = aEventTarget;
         return NS_OK;
       }
 
       // We have the remote inputStream, let's check if we can execute the
       // callback.
       case eRunning: {
-        if (mInputStreamCallback && aCallback) {
+        if (NS_WARN_IF(mInputStreamCallback && aCallback &&
+                       mInputStreamCallback != aCallback)) {
           return NS_ERROR_FAILURE;
         }
 
         nsresult rv = EnsureAsyncRemoteStream(lock);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
 
@@ -449,17 +451,18 @@ RemoteLazyInputStream::AsyncWait(nsIInpu
         asyncRemoteStream = mAsyncRemoteStream;
         break;
       }
 
       case eClosed:
         [[fallthrough]];
       default:
         MOZ_ASSERT(mState == eClosed);
-        if (mInputStreamCallback && aCallback) {
+        if (NS_WARN_IF(mInputStreamCallback && aCallback &&
+                       mInputStreamCallback != aCallback)) {
           return NS_ERROR_FAILURE;
         }
         break;
     }
   }
 
   if (asyncRemoteStream) {
     return asyncRemoteStream->AsyncWait(aCallback ? this : nullptr, 0, 0,
--- a/dom/file/uri/BlobURLInputStream.cpp
+++ b/dom/file/uri/BlobURLInputStream.cpp
@@ -135,17 +135,18 @@ NS_IMETHODIMP BlobURLInputStream::AsyncW
   MutexAutoLock lock(mStateMachineMutex);
 
   if (mState == State::ERROR) {
     MOZ_ASSERT(NS_FAILED(mError));
     return NS_ERROR_FAILURE;
   }
 
   // Pre-empting a valid callback with another is not allowed.
-  if (NS_WARN_IF(mAsyncWaitCallback && aCallback)) {
+  if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                 mAsyncWaitCallback != aCallback)) {
     return NS_ERROR_FAILURE;
   }
 
   mAsyncWaitTarget = aEventTarget;
   mAsyncWaitRequestedCount = aRequestedCount;
   mAsyncWaitFlags = aFlags;
   mAsyncWaitCallback = aCallback;
 
--- a/netwerk/base/PartiallySeekableInputStream.cpp
+++ b/netwerk/base/PartiallySeekableInputStream.cpp
@@ -246,17 +246,18 @@ PartiallySeekableInputStream::AsyncWait(
       }
     }
 
     return NS_OK;
   }
 
   {
     MutexAutoLock lock(mMutex);
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
     mAsyncWaitCallback = aCallback;
   }
 
   NS_ENSURE_STATE(mWeakAsyncInputStream);
   nsCOMPtr<nsIInputStreamCallback> callback = aCallback ? this : nullptr;
--- a/netwerk/base/nsBufferedStreams.cpp
+++ b/netwerk/base/nsBufferedStreams.cpp
@@ -671,17 +671,18 @@ nsBufferedInputStream::AsyncWait(nsIInpu
     aCallback->OnInputStreamReady(this);
     return NS_OK;
   }
 
   nsCOMPtr<nsIInputStreamCallback> callback = aCallback ? this : nullptr;
   {
     MutexAutoLock lock(mMutex);
 
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
     mAsyncWaitCallback = aCallback;
   }
 
   return stream->AsyncWait(callback, aFlags, aRequestedCount, aEventTarget);
 }
--- a/netwerk/base/nsMIMEInputStream.cpp
+++ b/netwerk/base/nsMIMEInputStream.cpp
@@ -265,17 +265,18 @@ nsMIMEInputStream::AsyncWait(nsIInputStr
   nsCOMPtr<nsIAsyncInputStream> asyncStream = do_QueryInterface(mStream);
   if (NS_WARN_IF(!asyncStream)) {
     return NS_ERROR_FAILURE;
   }
 
   nsCOMPtr<nsIInputStreamCallback> callback = aCallback ? this : nullptr;
   {
     MutexAutoLock lock(mMutex);
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
     mAsyncWaitCallback = aCallback;
   }
 
   return asyncStream->AsyncWait(callback, aFlags, aRequestedCount,
                                 aEventTarget);
--- a/netwerk/base/nsStreamTransportService.cpp
+++ b/netwerk/base/nsStreamTransportService.cpp
@@ -202,17 +202,18 @@ nsInputStreamTransport::AsyncWait(nsIInp
                                   nsIEventTarget* aEventTarget) {
   NS_ENSURE_STATE(!!mAsyncSource);
 
   nsCOMPtr<nsIInputStreamCallback> callback = aCallback ? this : nullptr;
 
   {
     MutexAutoLock lock(mMutex);
 
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
     mAsyncWaitCallback = aCallback;
   }
 
   return mAsyncSource->AsyncWait(callback, aFlags, aRequestedCount,
                                  aEventTarget);
--- a/xpcom/io/InputStreamLengthWrapper.cpp
+++ b/xpcom/io/InputStreamLengthWrapper.cpp
@@ -200,36 +200,26 @@ InputStreamLengthWrapper::CloseWithStatu
 
 NS_IMETHODIMP
 InputStreamLengthWrapper::AsyncWait(nsIInputStreamCallback* aCallback,
                                     uint32_t aFlags, uint32_t aRequestedCount,
                                     nsIEventTarget* aEventTarget) {
   NS_ENSURE_STATE(mInputStream);
   NS_ENSURE_STATE(mWeakAsyncInputStream);
 
-  nsCOMPtr<nsIInputStreamCallback> callback = this;
+  nsCOMPtr<nsIInputStreamCallback> callback = aCallback ? this : nullptr;
   {
     MutexAutoLock lock(mMutex);
 
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
-    bool hadCallback = !!mAsyncWaitCallback;
     mAsyncWaitCallback = aCallback;
-
-    if (!mAsyncWaitCallback) {
-      if (!hadCallback) {
-        // No pending operation.
-        return NS_OK;
-      }
-
-      // Abort current operation.
-      callback = nullptr;
-    }
   }
 
   return mWeakAsyncInputStream->AsyncWait(callback, aFlags, aRequestedCount,
                                           aEventTarget);
 }
 
 // nsIInputStreamCallback
 
--- a/xpcom/io/SlicedInputStream.cpp
+++ b/xpcom/io/SlicedInputStream.cpp
@@ -302,17 +302,18 @@ SlicedInputStream::AsyncWait(nsIInputStr
   nsCOMPtr<nsIInputStreamCallback> callback = aCallback ? this : nullptr;
 
   uint32_t flags = aFlags;
   uint32_t requestedCount = aRequestedCount;
 
   {
     MutexAutoLock lock(mMutex);
 
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
     mAsyncWaitCallback = aCallback;
 
     // If we haven't started retrieving data, let's see if we can seek.
     // If we cannot seek, we will do consecutive reads.
     if (mCurPos < mStart && mWeakSeekableInputStream) {
--- a/xpcom/io/nsMultiplexInputStream.cpp
+++ b/xpcom/io/nsMultiplexInputStream.cpp
@@ -815,17 +815,18 @@ nsMultiplexInputStream::AsyncWait(nsIInp
   {
     MutexAutoLock lock(mLock);
 
     // We must execute the callback also when the stream is closed.
     if (NS_FAILED(mStatus) && mStatus != NS_BASE_STREAM_CLOSED) {
       return mStatus;
     }
 
-    if (mAsyncWaitCallback && aCallback) {
+    if (NS_WARN_IF(mAsyncWaitCallback && aCallback &&
+                   mAsyncWaitCallback != aCallback)) {
       return NS_ERROR_FAILURE;
     }
 
     mAsyncWaitCallback = aCallback;
     mAsyncWaitFlags = aFlags;
     mAsyncWaitRequestedCount = aRequestedCount;
     mAsyncWaitEventTarget = aEventTarget;