Bug 1397595 P2 Delay removing nsPipeInputStream from nsPipe list until DrainInputStream() actually runs. r=froydnj
authorBen Kelly <ben@wanderview.com>
Tue, 12 Sep 2017 06:28:58 -0700
changeset 429787 3e638bc2c4f321d442e2bb13e89985bf921f4421
parent 429786 59bb8f38be8d4104b84c9c811e0f87b7242f1028
child 429788 061de9b749ef47e75cb59a424b7d07bd95a7998b
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1397595
milestone57.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 1397595 P2 Delay removing nsPipeInputStream from nsPipe list until DrainInputStream() actually runs. r=froydnj
xpcom/io/nsPipe3.cpp
--- a/xpcom/io/nsPipe3.cpp
+++ b/xpcom/io/nsPipe3.cpp
@@ -826,22 +826,32 @@ nsPipe::DrainInputStream(nsPipeReadState
 
     // Don't bother checking if this results in an advance buffer segment
     // read.  Since we are draining the entire stream we will read an
     // advance buffer segment no matter what.
     AdvanceReadSegment(aReadState, mon);
   }
 
   // Force the stream into an empty state.  Make sure mAvailable, mCursor, and
-  // mReadLimit are consistent with one another.  This is safe to do because
-  // we are always effectively removed from the mInputList after we are drained.
+  // mReadLimit are consistent with one another.
   aReadState.mAvailable = 0;
   aReadState.mReadCursor = nullptr;
   aReadState.mReadLimit = nullptr;
 
+  // Remove the input stream from the pipe's list of streams.  This will
+  // prevent the pipe from holding the stream alive or trying to update
+  // its read state any further.
+  DebugOnly<uint32_t> numRemoved = 0;
+  mInputList.RemoveElementsBy([&](nsPipeInputStream* aEntry) {
+    bool result = &aReadState == &aEntry->ReadState();
+    numRemoved += result ? 1 : 0;
+    return result;
+  });
+  MOZ_ASSERT(numRemoved == 1);
+
   // If we have read any segments from the advance buffer then we can
   // potentially notify blocked writers.
   if (!IsAdvanceBufferFull(mon) &&
       mOutput.OnOutputWritable(aEvents) == NotifyMonitor) {
     mon.NotifyAll();
   }
 }
 
@@ -973,17 +983,16 @@ 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,
                                                              mon);
-      mInputList.RemoveElementAt(i);
 
       // Notify after element is removed in case we re-enter as a result.
       if (action == NotifyMonitor) {
         mon.NotifyAll();
       }
 
       return;
     }
@@ -1004,31 +1013,30 @@ nsPipe::OnPipeException(nsresult aReason
     if (NS_FAILED(mStatus)) {
       return;
     }
 
     mStatus = aReason;
 
     bool needNotify = false;
 
-    nsTArray<nsPipeInputStream*> tmpInputList;
-    for (uint32_t i = 0; i < mInputList.Length(); ++i) {
+    // OnInputException() can drain the stream and remove it from
+    // mInputList.  So iterate over a temp list instead.
+    nsTArray<nsPipeInputStream*> list(mInputList);
+    for (uint32_t i = 0; i < list.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]);
+      if (aOutputOnly && list[i]->Available()) {
         continue;
       }
 
-      if (mInputList[i]->OnInputException(aReason, events, mon)
-          == NotifyMonitor) {
+      if (list[i]->OnInputException(aReason, events, mon) == NotifyMonitor) {
         needNotify = true;
       }
     }
-    mInputList = tmpInputList;
 
     if (mOutput.OnOutputException(aReason, events) == NotifyMonitor) {
       needNotify = true;
     }
 
     // Notify after we have removed any input streams from mInputList
     if (needNotify) {
       mon.NotifyAll();