Bug 1754031 - AsyncWait implementations should support updating flags on an existing listener, r=asuth,necko-reviewers,dragana
☠☠ backed out by 31b84a1b36f6 ☠ ☠
authorNika Layzell <nika@thelayzells.com>
Mon, 02 May 2022 20:44:20 +0000
changeset 615831 2541d8face65a3e2eb034c82e2bef008c37958d1
parent 615830 172227c66f574f5370fa37c749060a2ad273bce7
child 615832 bd3a2fcf4f0c7c48982c160a8e7911f8969af38c
push id162040
push usernlayzell@mozilla.com
push dateMon, 02 May 2022 20:47:54 +0000
treeherderautoland@110a8dce4b5d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, necko-reviewers, dragana
bugs1754031
milestone101.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;