Bug 1417448 - Better use of monitors in NS_ReadInputStreamToBuffer, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 20 Nov 2017 15:18:26 +0100
changeset 392670 2173819ac195dca869a9ae026c18bed598466c64
parent 392669 6d2e96805365cd287128980ddbc3a17b52c64393
child 392671 bb9035ea4c09d304f62af7f9613fbdde50eb15b2
push id97497
push useramarchesini@mozilla.com
push dateMon, 20 Nov 2017 14:18:55 +0000
treeherdermozilla-inbound@2173819ac195 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1417448
milestone59.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 1417448 - Better use of monitors in NS_ReadInputStreamToBuffer, r=smaug
netwerk/base/nsNetUtil.cpp
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@@ -1426,17 +1426,17 @@ public:
     BufferWriter(nsIInputStream* aInputStream,
                  void* aBuffer, int64_t aCount)
         : Runnable("BufferWriterRunnable")
         , mMonitor("BufferWriter.mMonitor")
         , mInputStream(aInputStream)
         , mBuffer(aBuffer)
         , mCount(aCount)
         , mWrittenData(0)
-        , mBufferType(mBuffer ? eExternal : eInternal)
+        , mBufferType(aBuffer ? eExternal : eInternal)
         , mAsyncResult(NS_OK)
         , mBufferSize(0)
     {
         MOZ_ASSERT(aInputStream);
         MOZ_ASSERT(aCount == -1 || aCount > 0);
         MOZ_ASSERT_IF(mBuffer, aCount > 0);
     }
 
@@ -1472,18 +1472,22 @@ public:
         return mWrittenData;
     }
 
     void*
     StealBuffer()
     {
         MOZ_ASSERT(mBufferType == eInternal);
 
+        MonitorAutoLock lock(mMonitor);
+
         void* buffer = mBuffer;
+
         mBuffer = nullptr;
+        mBufferSize = 0;
 
         return buffer;
     }
 
 private:
     ~BufferWriter()
     {
         if (mBuffer && mBufferType == eInternal) {
@@ -1546,105 +1550,106 @@ private:
         mTaskQueue = new TaskQueue(target.forget());
 
         MonitorAutoLock lock(mMonitor);
 
         nsCOMPtr<nsIRunnable> runnable = this;
         nsresult rv = mTaskQueue->Dispatch(runnable.forget());
         NS_ENSURE_SUCCESS(rv, rv);
 
-        lock.Wait();
+        rv = lock.Wait();
+        NS_ENSURE_SUCCESS(rv, rv);
 
         return mAsyncResult;
     }
 
     // This method runs on the I/O Thread when the owning thread is blocked by
     // the mMonitor. It is called multiple times until mCount is greater than 0
     // or until there is something to read in the stream.
     NS_IMETHOD
     Run() override
     {
         MOZ_ASSERT(mAsyncInputStream);
         MOZ_ASSERT(!mInputStream);
 
+        MonitorAutoLock lock(mMonitor);
+
         if (mCount == 0) {
-            OperationCompleted(NS_OK);
+            OperationCompleted(lock, NS_OK);
             return NS_OK;
         }
 
         if (mCount == -1 && !MaybeExpandBufferSize()) {
-            OperationCompleted(NS_ERROR_OUT_OF_MEMORY);
+            OperationCompleted(lock, NS_ERROR_OUT_OF_MEMORY);
             return NS_OK;
         }
 
         uint64_t offset = mWrittenData;
         uint64_t length = mCount == -1 ? BUFFER_SIZE : mCount;
 
         // Let's try to read it directly.
         uint32_t writtenData;
         nsresult rv = mAsyncInputStream->ReadSegments(NS_CopySegmentToBuffer,
                                                      static_cast<char*>(mBuffer) + offset,
                                                      length, &writtenData);
 
         // Operation completed.
         if (NS_SUCCEEDED(rv) && writtenData == 0) {
-            OperationCompleted(NS_OK);
+            OperationCompleted(lock, NS_OK);
             return NS_OK;
         }
 
         // If we succeeded, let's try to read again.
         if (NS_SUCCEEDED(rv)) {
             mWrittenData += writtenData;
             if (mCount != -1) {
                 MOZ_ASSERT(mCount >= writtenData);
                 mCount -= writtenData;
             }
 
             nsCOMPtr<nsIRunnable> runnable = this;
             rv = mTaskQueue->Dispatch(runnable.forget());
             if (NS_WARN_IF(NS_FAILED(rv))) {
-                OperationCompleted(rv);
+                OperationCompleted(lock, rv);
             }
         
             return NS_OK;
         }
 
         // Async wait...
         if (rv == NS_BASE_STREAM_WOULD_BLOCK) {
             rv = mAsyncInputStream->AsyncWait(this, 0, length, mTaskQueue);
             if (NS_WARN_IF(NS_FAILED(rv))) {
-                OperationCompleted(rv);
+                OperationCompleted(lock, rv);
             }
             return NS_OK;
         }
 
         // Error.
-        OperationCompleted(rv);
+        OperationCompleted(lock, rv);
         return NS_OK;
     }
 
     NS_IMETHOD
     OnInputStreamReady(nsIAsyncInputStream* aStream) override
     {
         MOZ_ASSERT(aStream == mAsyncInputStream);
         // The stream is ready, let's read it again.
         return Run();
     }
 
     // This function is called from the I/O thread and it will unblock the
     // owning thread.
     void
-    OperationCompleted(nsresult aRv)
+    OperationCompleted(MonitorAutoLock& aLock, nsresult aRv)
     {
-        MonitorAutoLock lock(mMonitor);
-
         mAsyncResult = aRv;
 
         // This will unlock the owning thread.
-        lock.Notify();
+        aLock.Notify();
     }
 
     bool
     MaybeExpandBufferSize()
     {
         MOZ_ASSERT(mCount == -1);
 
         if (mBufferSize >= mWrittenData + BUFFER_SIZE) {