Bug 1417448 - Better use of monitors in NS_ReadInputStreamToBuffer, r=smaug a=gchang
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 20 Nov 2017 15:18:26 +0100
changeset 442576 b4a737a2a3d7f56cace147a0fbaaf558b11a8f60
parent 442575 a9ce90a8eeabc58651c4e39c6a16be095865ac90
child 442577 40a6d6c3807248bae0430bc57444a9aeaf0bbad3
push id8260
push userarchaeopteryx@coole-files.de
push dateWed, 29 Nov 2017 14:14:30 +0000
treeherdermozilla-beta@40a6d6c38072 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, gchang
bugs1417448
milestone58.0
Bug 1417448 - Better use of monitors in NS_ReadInputStreamToBuffer, r=smaug a=gchang
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) {
@@ -1547,106 +1551,107 @@ private:
 
         MonitorAutoLock lock(mMonitor);
 
         nsCOMPtr<nsIRunnable> runnable = this;
         nsresult rv = mTaskQueue->Dispatch(runnable.forget(),
                                            AbstractThread::DontAssertDispatchSuccess);
         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(),
                                       AbstractThread::DontAssertDispatchSuccess);
             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) {