Bug 1136762 - TSan: data race xpcom/io/nsPipe3.cpp:1061 CloseWithStatus. r=nfroyd.
authorJulian Seward <jseward@acm.org>
Wed, 17 Feb 2016 11:22:28 +0100
changeset 307552 5271840b178c83f3021833252c6595d600fcc6c8
parent 307551 8ac1b6ca916a6589e8ea7a284a6068bf70d03a95
child 307553 9e71a38057d1e37194c6a367d7df337e7088acca
push id9214
push userraliiev@mozilla.com
push dateMon, 07 Mar 2016 14:25:21 +0000
treeherdermozilla-aurora@8849dd1a4a79 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs1136762
milestone47.0a1
Bug 1136762 - TSan: data race xpcom/io/nsPipe3.cpp:1061 CloseWithStatus. r=nfroyd.
xpcom/io/nsPipe3.cpp
--- a/xpcom/io/nsPipe3.cpp
+++ b/xpcom/io/nsPipe3.cpp
@@ -179,39 +179,48 @@ public:
     mBlocking = !aNonBlocking;
   }
 
   uint32_t Available();
 
   // synchronously wait for the pipe to become readable.
   nsresult Wait();
 
-  MonitorAction OnInputReadable(uint32_t aBytesWritten, nsPipeEvents&);
-  MonitorAction OnInputException(nsresult, nsPipeEvents&);
+  // These two don't acquire the monitor themselves.  Instead they
+  // expect their caller to have done so and to pass the monitor as
+  // evidence.
+  MonitorAction OnInputReadable(uint32_t aBytesWritten, nsPipeEvents&,
+                                const ReentrantMonitorAutoEnter& ev);
+  MonitorAction OnInputException(nsresult, nsPipeEvents&,
+                                 const ReentrantMonitorAutoEnter& ev);
 
   nsPipeReadState& ReadState()
   {
     return mReadState;
   }
 
   const nsPipeReadState& ReadState() const
   {
     return mReadState;
   }
 
   nsresult Status() const;
 
+  // A version of Status() that doesn't acquire the monitor.
+  nsresult Status(const ReentrantMonitorAutoEnter& ev) const;
+
 private:
   virtual ~nsPipeInputStream();
 
   RefPtr<nsPipe>               mPipe;
 
   int64_t                        mLogicalOffset;
   // Individual input streams can be closed without effecting the rest of the
-  // pipe.  So track individual input stream status separately.
+  // pipe.  So track individual input stream status separately.  |mInputStatus|
+  // is protected by |mPipe->mReentrantMonitor|.
   nsresult                       mInputStatus;
   bool                           mBlocking;
 
   // these variables can only be accessed while inside the pipe's monitor
   bool                           mBlocked;
   nsCOMPtr<nsIInputStreamCallback> mCallback;
   uint32_t                       mCallbackFlags;
 
@@ -352,16 +361,17 @@ private:
 
   ReentrantMonitor    mReentrantMonitor;
   nsSegmentedBuffer   mBuffer;
 
   int32_t             mWriteSegment;
   char*               mWriteCursor;
   char*               mWriteLimit;
 
+  // |mStatus| is protected by |mReentrantMonitor|.
   nsresult            mStatus;
   bool                mInited;
 };
 
 //-----------------------------------------------------------------------------
 
 // RAII class representing an active read segment.  When it goes out of scope
 // it automatically updates the read cursor and releases the read segment.
@@ -517,17 +527,20 @@ nsPipe::Release()
 {
   MOZ_ASSERT(int32_t(mRefCnt) > 0, "dup release");
   nsrefcnt count = --mRefCnt;
   NS_LOG_RELEASE(this, count, "nsPipe");
   if (count == 0) {
     delete (this);
     return 0;
   }
-  if (mOriginalInput && count == 1) {
+  // Avoid racing on |mOriginalInput| by only looking at it when
+  // the refcount is 1, that is, we are the only pointer (hence only
+  // thread) to access it.
+  if (count == 1 && mOriginalInput) {
     mOriginalInput = nullptr;
     return 1;
   }
   return count;
 }
 
 NS_IMETHODIMP
 nsPipe::Init(bool aNonBlockingIn,
@@ -861,17 +874,18 @@ nsPipe::AdvanceWriteCursor(uint32_t aByt
       if (mBuffer.GetSize() >= mBuffer.GetMaxSize()) {
         mOutput.SetWritable(false);
       }
     }
 
     // notify input stream that pipe now contains additional data
     bool needNotify = false;
     for (uint32_t i = 0; i < mInputList.Length(); ++i) {
-      if (mInputList[i]->OnInputReadable(aBytesWritten, events) == NotifyMonitor) {
+      if (mInputList[i]->OnInputReadable(aBytesWritten, events, mon)
+          == NotifyMonitor) {
         needNotify = true;
       }
     }
 
     if (needNotify) {
       mon.NotifyAll();
     }
   }
@@ -902,17 +916,18 @@ nsPipe::OnInputStreamException(nsPipeInp
     }
 
     // Otherwise just close the particular stream that hit an exception.
     for (uint32_t i = 0; i < mInputList.Length(); ++i) {
       if (mInputList[i] != aStream) {
         continue;
       }
 
-      MonitorAction action = mInputList[i]->OnInputException(aReason, events);
+      MonitorAction action = mInputList[i]->OnInputException(aReason, events,
+                                                             mon);
       mInputList.RemoveElementAt(i);
 
       // Notify after element is removed in case we re-enter as a result.
       if (action == NotifyMonitor) {
         mon.NotifyAll();
       }
 
       return;
@@ -943,17 +958,18 @@ nsPipe::OnPipeException(nsresult aReason
     for (uint32_t i = 0; i < mInputList.Length(); ++i) {
       // an output-only exception applies to the input end if the pipe has
       // zero bytes available.
       if (aOutputOnly && mInputList[i]->Available()) {
         tmpInputList.AppendElement(mInputList[i]);
         continue;
       }
 
-      if (mInputList[i]->OnInputException(aReason, events) == NotifyMonitor) {
+      if (mInputList[i]->OnInputException(aReason, events, mon)
+          == NotifyMonitor) {
         needNotify = true;
       }
     }
     mInputList = tmpInputList;
 
     if (mOutput.OnOutputException(aReason, events) == NotifyMonitor) {
       needNotify = true;
     }
@@ -1131,32 +1147,34 @@ nsPipeInputStream::Available()
 
 nsresult
 nsPipeInputStream::Wait()
 {
   NS_ASSERTION(mBlocking, "wait on non-blocking pipe input stream");
 
   ReentrantMonitorAutoEnter mon(mPipe->mReentrantMonitor);
 
-  while (NS_SUCCEEDED(Status()) && (mReadState.mAvailable == 0)) {
+  while (NS_SUCCEEDED(Status(mon)) && (mReadState.mAvailable == 0)) {
     LOG(("III pipe input: waiting for data\n"));
 
     mBlocked = true;
     mon.Wait();
     mBlocked = false;
 
     LOG(("III pipe input: woke up [status=%x available=%u]\n",
-         Status(), mReadState.mAvailable));
+         Status(mon), mReadState.mAvailable));
   }
 
-  return Status() == NS_BASE_STREAM_CLOSED ? NS_OK : Status();
+  return Status(mon) == NS_BASE_STREAM_CLOSED ? NS_OK : Status(mon);
 }
 
 MonitorAction
-nsPipeInputStream::OnInputReadable(uint32_t aBytesWritten, nsPipeEvents& aEvents)
+nsPipeInputStream::OnInputReadable(uint32_t aBytesWritten,
+                                   nsPipeEvents& aEvents,
+                                   const ReentrantMonitorAutoEnter& ev)
 {
   MonitorAction result = DoNotNotifyMonitor;
 
   mPipe->mReentrantMonitor.AssertCurrentThreadIn();
   mReadState.mAvailable += aBytesWritten;
 
   if (mCallback && !(mCallbackFlags & WAIT_CLOSURE_ONLY)) {
     aEvents.NotifyInputReady(this, mCallback);
@@ -1165,17 +1183,18 @@ nsPipeInputStream::OnInputReadable(uint3
   } else if (mBlocked) {
     result = NotifyMonitor;
   }
 
   return result;
 }
 
 MonitorAction
-nsPipeInputStream::OnInputException(nsresult aReason, nsPipeEvents& aEvents)
+nsPipeInputStream::OnInputException(nsresult aReason, nsPipeEvents& aEvents,
+                                    const ReentrantMonitorAutoEnter& ev)
 {
   LOG(("nsPipeInputStream::OnInputException [this=%x reason=%x]\n",
        this, aReason));
 
   MonitorAction result = DoNotNotifyMonitor;
 
   NS_ASSERTION(NS_FAILED(aReason), "huh? successful exception");
 
@@ -1197,16 +1216,18 @@ nsPipeInputStream::OnInputException(nsre
   return result;
 }
 
 NS_IMETHODIMP
 nsPipeInputStream::CloseWithStatus(nsresult aReason)
 {
   LOG(("III CloseWithStatus [this=%x reason=%x]\n", this, aReason));
 
+  ReentrantMonitorAutoEnter mon(mPipe->mReentrantMonitor);
+
   if (NS_FAILED(mInputStatus)) {
     return NS_OK;
   }
 
   if (NS_SUCCEEDED(aReason)) {
     aReason = NS_BASE_STREAM_CLOSED;
   }
 
@@ -1222,18 +1243,18 @@ nsPipeInputStream::Close()
 
 NS_IMETHODIMP
 nsPipeInputStream::Available(uint64_t* aResult)
 {
   // nsPipeInputStream supports under 4GB stream only
   ReentrantMonitorAutoEnter mon(mPipe->mReentrantMonitor);
 
   // return error if closed
-  if (!mReadState.mAvailable && NS_FAILED(Status())) {
-    return Status();
+  if (!mReadState.mAvailable && NS_FAILED(Status(mon))) {
+    return Status(mon);
   }
 
   *aResult = (uint64_t)mReadState.mAvailable;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPipeInputStream::ReadSegments(nsWriteSegmentFun aWriter,
@@ -1335,17 +1356,17 @@ nsPipeInputStream::AsyncWait(nsIInputStr
     }
 
     nsCOMPtr<nsIInputStreamCallback> proxy;
     if (aTarget) {
       proxy = NS_NewInputStreamReadyEvent(aCallback, aTarget);
       aCallback = proxy;
     }
 
-    if (NS_FAILED(Status()) ||
+    if (NS_FAILED(Status(mon)) ||
        (mReadState.mAvailable && !(aFlags & WAIT_CLOSURE_ONLY))) {
       // stream is already closed or readable; post event.
       pipeEvents.NotifyInputReady(this, aCallback);
     } else {
       // queue up callback object to be notified when data becomes available
       mCallback = aCallback;
       mCallbackFlags = aFlags;
     }
@@ -1361,18 +1382,18 @@ nsPipeInputStream::Seek(int32_t aWhence,
 }
 
 NS_IMETHODIMP
 nsPipeInputStream::Tell(int64_t* aOffset)
 {
   ReentrantMonitorAutoEnter mon(mPipe->mReentrantMonitor);
 
   // return error if closed
-  if (!mReadState.mAvailable && NS_FAILED(Status())) {
-    return Status();
+  if (!mReadState.mAvailable && NS_FAILED(Status(mon))) {
+    return Status(mon);
   }
 
   *aOffset = mLogicalOffset;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPipeInputStream::SetEOF()
@@ -1475,19 +1496,26 @@ nsPipeInputStream::GetCloneable(bool* aC
 
 NS_IMETHODIMP
 nsPipeInputStream::Clone(nsIInputStream** aCloneOut)
 {
   return mPipe->CloneInputStream(this, aCloneOut);
 }
 
 nsresult
+nsPipeInputStream::Status(const ReentrantMonitorAutoEnter& ev) const
+{
+  return NS_FAILED(mInputStatus) ? mInputStatus : mPipe->mStatus;
+}
+
+nsresult
 nsPipeInputStream::Status() const
 {
-  return NS_FAILED(mInputStatus) ? mInputStatus : mPipe->mStatus;
+  ReentrantMonitorAutoEnter mon(mPipe->mReentrantMonitor);
+  return Status(mon);
 }
 
 nsPipeInputStream::~nsPipeInputStream()
 {
   Close();
 }
 
 //-----------------------------------------------------------------------------