Bug 1279246 - Hang due to CacheFileInputStream deadlock, r=honzab
authorMichal Novotny <michal.novotny@gmail.com>
Tue, 12 Jul 2016 17:58:38 +0200
changeset 304779 8446a3f5f3c24a75f76e8864b11c4dd6af1bd7ce
parent 304778 3c622421db18541c3ab48f2dcee67bb00d2a67ad
child 304780 c6bd7036ef52385be6a85afbf6836548129cd35a
push id30441
push usercbook@mozilla.com
push dateWed, 13 Jul 2016 15:27:54 +0000
treeherdermozilla-central@5fc004cf355f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs1279246
milestone50.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 1279246 - Hang due to CacheFileInputStream deadlock, r=honzab
netwerk/cache2/CacheFile.cpp
netwerk/cache2/CacheFileChunk.cpp
netwerk/cache2/CacheFileChunk.h
netwerk/cache2/CacheFileInputStream.cpp
netwerk/cache2/CacheFileInputStream.h
netwerk/cache2/CacheFileOutputStream.cpp
netwerk/test/unit/test_bug1279246.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/cache2/CacheFile.cpp
+++ b/netwerk/cache2/CacheFile.cpp
@@ -1535,28 +1535,28 @@ CacheFile::BytesFromChunk(uint32_t aInde
   }
 
   uint32_t i;
   for (i = aIndex; i <= maxPreloadedChunk; ++i) {
     CacheFileChunk * chunk;
 
     chunk = mChunks.GetWeak(i);
     if (chunk) {
-      MOZ_ASSERT(i == lastChunk || chunk->mDataSize == kChunkSize);
+      MOZ_ASSERT(i == lastChunk || chunk->DataSize() == kChunkSize);
       if (chunk->IsReady()) {
         continue;
       }
 
       // don't search this chunk in cached
       break;
     }
 
     chunk = mCachedChunks.GetWeak(i);
     if (chunk) {
-      MOZ_ASSERT(i == lastChunk || chunk->mDataSize == kChunkSize);
+      MOZ_ASSERT(i == lastChunk || chunk->DataSize() == kChunkSize);
       continue;
     }
 
     break;
   }
 
   // theoretic bytes in advance
   int64_t advance = int64_t(i - aIndex) * kChunkSize;
@@ -1945,26 +1945,26 @@ CacheFile::PadChunkWithZeroes(uint32_t a
   nsresult rv;
   RefPtr<CacheFileChunk> chunk;
   rv = GetChunkLocked(aChunkIdx, WRITER, nullptr, getter_AddRefs(chunk));
   NS_ENSURE_SUCCESS(rv, rv);
 
   LOG(("CacheFile::PadChunkWithZeroes() - Zeroing hole in chunk %d, range %d-%d"
        " [this=%p]", aChunkIdx, chunk->DataSize(), kChunkSize - 1, this));
 
-  rv = chunk->EnsureBufSize(kChunkSize);
-  if (NS_FAILED(rv)) {
+  CacheFileChunkWriteHandle hnd = chunk->GetWriteHandle(kChunkSize);
+  if (!hnd.Buf()) {
     ReleaseOutsideLock(chunk.forget());
-    SetError(rv);
-    return rv;
+    SetError(NS_ERROR_OUT_OF_MEMORY);
+    return NS_ERROR_OUT_OF_MEMORY;
   }
-  memset(chunk->BufForWriting() + chunk->DataSize(), 0, kChunkSize - chunk->DataSize());
 
-  chunk->UpdateDataSize(chunk->DataSize(), kChunkSize - chunk->DataSize(),
-                        false);
+  uint32_t offset = hnd.DataSize();
+  memset(hnd.Buf() + offset, 0, kChunkSize - offset);
+  hnd.UpdateDataSize(offset, kChunkSize - offset);
 
   ReleaseOutsideLock(chunk.forget());
 
   return NS_OK;
 }
 
 void
 CacheFile::SetError(nsresult aStatus)
--- a/netwerk/cache2/CacheFileChunk.cpp
+++ b/netwerk/cache2/CacheFileChunk.cpp
@@ -8,16 +8,237 @@
 #include "CacheFile.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace net {
 
 #define kMinBufSize        512
 
+CacheFileChunkBuffer::CacheFileChunkBuffer(CacheFileChunk *aChunk)
+  : mChunk(aChunk)
+  , mBuf(nullptr)
+  , mBufSize(0)
+  , mDataSize(0)
+  , mReadHandlesCount(0)
+  , mWriteHandleExists(false)
+{
+}
+
+CacheFileChunkBuffer::~CacheFileChunkBuffer()
+{
+  if (mBuf) {
+    CacheFileUtils::FreeBuffer(mBuf);
+    mBuf = nullptr;
+    mChunk->BuffersAllocationChanged(mBufSize, 0);
+    mBufSize = 0;
+  }
+}
+
+void
+CacheFileChunkBuffer::CopyFrom(CacheFileChunkBuffer *aOther)
+{
+  MOZ_RELEASE_ASSERT(mBufSize >= aOther->mDataSize);
+  mDataSize = aOther->mDataSize;
+  memcpy(mBuf, aOther->mBuf, mDataSize);
+}
+
+nsresult
+CacheFileChunkBuffer::FillInvalidRanges(CacheFileChunkBuffer *aOther,
+                                        CacheFileUtils::ValidityMap *aMap)
+{
+  nsresult rv;
+
+  rv = EnsureBufSize(aOther->mDataSize);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  uint32_t invalidOffset = 0;
+  uint32_t invalidLength;
+
+  for (uint32_t i = 0; i < aMap->Length(); ++i) {
+    uint32_t validOffset = (*aMap)[i].Offset();
+    uint32_t validLength = (*aMap)[i].Len();
+
+    MOZ_RELEASE_ASSERT(invalidOffset <= validOffset);
+    invalidLength = validOffset - invalidOffset;
+    if (invalidLength > 0) {
+      MOZ_RELEASE_ASSERT(invalidOffset + invalidLength <= aOther->mBufSize);
+      memcpy(mBuf + invalidOffset, aOther->mBuf + invalidOffset, invalidLength);
+    }
+    invalidOffset = validOffset + validLength;
+  }
+
+  if (invalidOffset < aOther->mBufSize) {
+    invalidLength = aOther->mBufSize - invalidOffset;
+    memcpy(mBuf + invalidOffset, aOther->mBuf + invalidOffset, invalidLength);
+  }
+
+  return NS_OK;
+}
+
+MOZ_MUST_USE nsresult
+CacheFileChunkBuffer::EnsureBufSize(uint32_t aBufSize)
+{
+  AssertOwnsLock();
+
+  if (mBufSize >= aBufSize) {
+    return NS_OK;
+  }
+
+  // find smallest power of 2 greater than or equal to aBufSize
+  aBufSize--;
+  aBufSize |= aBufSize >> 1;
+  aBufSize |= aBufSize >> 2;
+  aBufSize |= aBufSize >> 4;
+  aBufSize |= aBufSize >> 8;
+  aBufSize |= aBufSize >> 16;
+  aBufSize++;
+
+  const uint32_t minBufSize = kMinBufSize;
+  const uint32_t maxBufSize = kChunkSize;
+  aBufSize = clamped(aBufSize, minBufSize, maxBufSize);
+
+  if (!mChunk->CanAllocate(aBufSize - mBufSize)) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  char *newBuf = static_cast<char *>(realloc(mBuf, aBufSize));
+  if (!newBuf) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  mChunk->BuffersAllocationChanged(mBufSize, aBufSize);
+  mBuf = newBuf;
+  mBufSize = aBufSize;
+
+  return NS_OK;
+}
+
+void
+CacheFileChunkBuffer::SetDataSize(uint32_t aDataSize)
+{
+  MOZ_RELEASE_ASSERT(
+    mDataSize <= mBufSize ||
+    (mBufSize == 0 && mChunk->mState == CacheFileChunk::READING));
+  mDataSize = aDataSize;
+}
+
+void
+CacheFileChunkBuffer::AssertOwnsLock() const
+{
+  mChunk->AssertOwnsLock();
+}
+
+void
+CacheFileChunkBuffer::RemoveReadHandle()
+{
+  AssertOwnsLock();
+  MOZ_RELEASE_ASSERT(mReadHandlesCount);
+  MOZ_RELEASE_ASSERT(!mWriteHandleExists);
+  mReadHandlesCount--;
+
+  if (mReadHandlesCount == 0 && mChunk->mBuf != this) {
+    DebugOnly<bool> removed = mChunk->mOldBufs.RemoveElement(this);
+    MOZ_ASSERT(removed);
+  }
+}
+
+void
+CacheFileChunkBuffer::RemoveWriteHandle()
+{
+  AssertOwnsLock();
+  MOZ_RELEASE_ASSERT(mReadHandlesCount == 0);
+  MOZ_RELEASE_ASSERT(mWriteHandleExists);
+  mWriteHandleExists = false;
+}
+
+size_t
+CacheFileChunkBuffer::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const
+{
+  size_t n = mallocSizeOf(this);
+
+  if (mBuf) {
+    n += mallocSizeOf(mBuf);
+  }
+
+  return n;
+}
+
+uint32_t
+CacheFileChunkHandle::DataSize()
+{
+  MOZ_ASSERT(mBuf, "Unexpected call on dummy handle");
+  mBuf->AssertOwnsLock();
+  return mBuf->mDataSize;
+}
+
+uint32_t
+CacheFileChunkHandle::Offset()
+{
+  MOZ_ASSERT(mBuf, "Unexpected call on dummy handle");
+  mBuf->AssertOwnsLock();
+  return mBuf->mChunk->Index() * kChunkSize;
+}
+
+CacheFileChunkReadHandle::CacheFileChunkReadHandle(CacheFileChunkBuffer *aBuf)
+{
+  mBuf = aBuf;
+  mBuf->mReadHandlesCount++;
+}
+
+CacheFileChunkReadHandle::~CacheFileChunkReadHandle()
+{
+  mBuf->RemoveReadHandle();
+}
+
+const char *
+CacheFileChunkReadHandle::Buf()
+{
+  return mBuf->mBuf;
+}
+
+CacheFileChunkWriteHandle::CacheFileChunkWriteHandle(CacheFileChunkBuffer *aBuf)
+{
+  mBuf = aBuf;
+  if (mBuf) {
+    MOZ_ASSERT(!mBuf->mWriteHandleExists);
+    mBuf->mWriteHandleExists = true;
+  }
+}
+
+CacheFileChunkWriteHandle::~CacheFileChunkWriteHandle()
+{
+  if (mBuf) {
+    mBuf->RemoveWriteHandle();
+  }
+}
+
+char *
+CacheFileChunkWriteHandle::Buf()
+{
+  return mBuf ? mBuf->mBuf : nullptr;
+}
+
+void
+CacheFileChunkWriteHandle::UpdateDataSize(uint32_t aOffset, uint32_t aLen)
+{
+  MOZ_ASSERT(mBuf, "Write performed on dummy handle?");
+  MOZ_ASSERT(aOffset <= mBuf->mDataSize);
+  MOZ_ASSERT(aOffset + aLen <= mBuf->mBufSize);
+
+  if (aOffset + aLen > mBuf->mDataSize) {
+    mBuf->mDataSize = aOffset + aLen;
+  }
+
+  mBuf->mChunk->UpdateDataSize(aOffset, aLen);
+}
+
+
 class NotifyUpdateListenerEvent : public Runnable {
 public:
   NotifyUpdateListenerEvent(CacheFileChunkListener *aCallback,
                             CacheFileChunk *aChunk)
     : mCallback(aCallback)
     , mChunk(aChunk)
   {
     LOG(("NotifyUpdateListenerEvent::NotifyUpdateListenerEvent() [this=%p]",
@@ -104,158 +325,149 @@ NS_INTERFACE_MAP_END_THREADSAFE
 CacheFileChunk::CacheFileChunk(CacheFile *aFile, uint32_t aIndex,
                                bool aInitByWriter)
   : CacheMemoryConsumer(aFile->mOpenAsMemoryOnly ? MEMORY_ONLY : DONT_REPORT)
   , mIndex(aIndex)
   , mState(INITIAL)
   , mStatus(NS_OK)
   , mIsDirty(false)
   , mActiveChunk(false)
-  , mDataSize(0)
-  , mReportedAllocation(0)
+  , mBuffersSize(0)
   , mLimitAllocation(!aFile->mOpenAsMemoryOnly && aInitByWriter)
   , mIsPriority(aFile->mPriority)
-  , mBuf(nullptr)
-  , mBufSize(0)
-  , mRWBuf(nullptr)
-  , mRWBufSize(0)
-  , mReadHash(0)
+  , mExpectedHash(0)
   , mFile(aFile)
 {
   LOG(("CacheFileChunk::CacheFileChunk() [this=%p, index=%u, initByWriter=%d]",
        this, aIndex, aInitByWriter));
   MOZ_COUNT_CTOR(CacheFileChunk);
+
+  mBuf = new CacheFileChunkBuffer(this);
 }
 
 CacheFileChunk::~CacheFileChunk()
 {
   LOG(("CacheFileChunk::~CacheFileChunk() [this=%p]", this));
   MOZ_COUNT_DTOR(CacheFileChunk);
+}
 
-  if (mBuf) {
-    CacheFileUtils::FreeBuffer(mBuf);
-    mBuf = nullptr;
-    mBufSize = 0;
-    ChunkAllocationChanged();
-  }
-
-  if (mRWBuf) {
-    CacheFileUtils::FreeBuffer(mRWBuf);
-    mRWBuf = nullptr;
-    mRWBufSize = 0;
-    ChunkAllocationChanged();
-  }
+void
+CacheFileChunk::AssertOwnsLock() const
+{
+  mFile->AssertOwnsLock();
 }
 
 void
 CacheFileChunk::InitNew()
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   LOG(("CacheFileChunk::InitNew() [this=%p]", this));
 
   MOZ_ASSERT(mState == INITIAL);
   MOZ_ASSERT(NS_SUCCEEDED(mStatus));
-  MOZ_ASSERT(!mBuf);
-  MOZ_ASSERT(!mRWBuf);
+  MOZ_ASSERT(!mBuf->Buf());
+  MOZ_ASSERT(!mWritingStateHandle);
+  MOZ_ASSERT(!mReadingStateBuf);
   MOZ_ASSERT(!mIsDirty);
-  MOZ_ASSERT(mDataSize == 0);
 
+  mBuf = new CacheFileChunkBuffer(this);
   mState = READY;
 }
 
 nsresult
 CacheFileChunk::Read(CacheFileHandle *aHandle, uint32_t aLen,
                      CacheHash::Hash16_t aHash,
                      CacheFileChunkListener *aCallback)
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   LOG(("CacheFileChunk::Read() [this=%p, handle=%p, len=%d, listener=%p]",
        this, aHandle, aLen, aCallback));
 
   MOZ_ASSERT(mState == INITIAL);
   MOZ_ASSERT(NS_SUCCEEDED(mStatus));
-  MOZ_ASSERT(!mBuf);
-  MOZ_ASSERT(!mRWBuf);
+  MOZ_ASSERT(!mBuf->Buf());
+  MOZ_ASSERT(!mWritingStateHandle);
+  MOZ_ASSERT(!mReadingStateBuf);
   MOZ_ASSERT(aLen);
 
   nsresult rv;
 
   mState = READING;
 
-  if (CanAllocate(aLen)) {
-    mRWBuf = static_cast<char *>(malloc(aLen));
-    if (mRWBuf) {
-      mRWBufSize = aLen;
-      ChunkAllocationChanged();
-    }
-  }
-
-  if (!mRWBuf) {
-    // Allocation was denied or failed
-    SetError(NS_ERROR_OUT_OF_MEMORY);
+  RefPtr<CacheFileChunkBuffer> tmpBuf = new CacheFileChunkBuffer(this);
+  rv = tmpBuf->EnsureBufSize(aLen);
+  if (NS_FAILED(rv)) {
+    SetError(rv);
     return mStatus;
   }
+  tmpBuf->SetDataSize(aLen);
 
-  DoMemoryReport(MemorySize());
-
-  rv = CacheFileIOManager::Read(aHandle, mIndex * kChunkSize, mRWBuf, aLen,
+  rv = CacheFileIOManager::Read(aHandle, mIndex * kChunkSize,
+                                tmpBuf->Buf(), aLen,
                                 this);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     rv = mIndex ? NS_ERROR_FILE_CORRUPTED : NS_ERROR_FILE_NOT_FOUND;
     SetError(rv);
   } else {
+    mReadingStateBuf.swap(tmpBuf);
     mListener = aCallback;
-    mDataSize = aLen;
-    mReadHash = aHash;
+    // mBuf contains no data but we set datasize to size of the data that will
+    // be read from the disk. No handle is allowed to access the non-existent
+    // data until reading finishes, but data can be appended or overwritten.
+    // These pieces are tracked in mValidityMap and will be merged with the data
+    // read from disk in OnDataRead().
+    mBuf->SetDataSize(aLen);
+    mExpectedHash = aHash;
   }
 
   return rv;
 }
 
 nsresult
 CacheFileChunk::Write(CacheFileHandle *aHandle,
                       CacheFileChunkListener *aCallback)
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   LOG(("CacheFileChunk::Write() [this=%p, handle=%p, listener=%p]",
        this, aHandle, aCallback));
 
   MOZ_ASSERT(mState == READY);
   MOZ_ASSERT(NS_SUCCEEDED(mStatus));
-  MOZ_ASSERT(!mRWBuf);
-  MOZ_ASSERT(mBuf);
-  MOZ_ASSERT(mDataSize); // Don't write chunk when it is empty
+  MOZ_ASSERT(!mWritingStateHandle);
+  MOZ_ASSERT(mBuf->DataSize()); // Don't write chunk when it is empty
+  MOZ_ASSERT(mBuf->ReadHandlesCount() == 0);
+  MOZ_ASSERT(!mBuf->WriteHandleExists());
 
   nsresult rv;
 
   mState = WRITING;
-  mRWBuf = mBuf;
-  mRWBufSize = mBufSize;
-  mBuf = nullptr;
-  mBufSize = 0;
+  mWritingStateHandle = new CacheFileChunkReadHandle(mBuf);
 
-  rv = CacheFileIOManager::Write(aHandle, mIndex * kChunkSize, mRWBuf,
-                                 mDataSize, false, false, this);
+  rv = CacheFileIOManager::Write(aHandle, mIndex * kChunkSize,
+                                 mWritingStateHandle->Buf(),
+                                 mWritingStateHandle->DataSize(),
+                                 false, false, this);
   if (NS_WARN_IF(NS_FAILED(rv))) {
+    mWritingStateHandle = nullptr;
     SetError(rv);
   } else {
     mListener = aCallback;
     mIsDirty = false;
   }
 
   return rv;
 }
 
 void
 CacheFileChunk::WaitForUpdate(CacheFileChunkListener *aCallback)
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   LOG(("CacheFileChunk::WaitForUpdate() [this=%p, listener=%p]",
        this, aCallback));
 
   MOZ_ASSERT(mFile->mOutput);
   MOZ_ASSERT(IsReady());
 
 #ifdef DEBUG
@@ -276,17 +488,17 @@ CacheFileChunk::WaitForUpdate(CacheFileC
   item->mCallback = aCallback;
 
   mUpdateListeners.AppendElement(item);
 }
 
 nsresult
 CacheFileChunk::CancelWait(CacheFileChunkListener *aCallback)
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   LOG(("CacheFileChunk::CancelWait() [this=%p, listener=%p]", this, aCallback));
 
   MOZ_ASSERT(IsReady());
 
   uint32_t i;
   for (i = 0 ; i < mUpdateListeners.Length() ; i++) {
     ChunkListenerItem *item = mUpdateListeners[i];
@@ -305,17 +517,17 @@ CacheFileChunk::CancelWait(CacheFileChun
 #endif
 
   return NS_OK;
 }
 
 nsresult
 CacheFileChunk::NotifyUpdateListeners()
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   LOG(("CacheFileChunk::NotifyUpdateListeners() [this=%p]", this));
 
   MOZ_ASSERT(IsReady());
 
   nsresult rv, rv2;
 
   rv = NS_OK;
@@ -334,65 +546,57 @@ CacheFileChunk::NotifyUpdateListeners()
   }
 
   mUpdateListeners.Clear();
 
   return rv;
 }
 
 uint32_t
-CacheFileChunk::Index()
+CacheFileChunk::Index() const
 {
   return mIndex;
 }
 
 CacheHash::Hash16_t
-CacheFileChunk::Hash()
+CacheFileChunk::Hash() const
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   MOZ_ASSERT(!mListener);
   MOZ_ASSERT(IsReady());
 
-  return CacheHash::Hash16(mDataSize ? BufForReading() : nullptr, mDataSize);
+  return CacheHash::Hash16(mBuf->Buf(), mBuf->DataSize());
 }
 
 uint32_t
-CacheFileChunk::DataSize()
+CacheFileChunk::DataSize() const
 {
-  mFile->AssertOwnsLock();
-  return mDataSize;
+  return mBuf->DataSize();
 }
 
 void
-CacheFileChunk::UpdateDataSize(uint32_t aOffset, uint32_t aLen, bool aEOF)
+CacheFileChunk::UpdateDataSize(uint32_t aOffset, uint32_t aLen)
 {
-  mFile->AssertOwnsLock();
-
-  MOZ_ASSERT(!aEOF, "Implement me! What to do with opened streams?");
-  MOZ_ASSERT(aOffset <= mDataSize);
-  MOZ_ASSERT(aLen != 0);
+  AssertOwnsLock();
 
   // UpdateDataSize() is called only when we've written some data to the chunk
   // and we never write data anymore once some error occurs.
   MOZ_ASSERT(NS_SUCCEEDED(mStatus));
 
-  LOG(("CacheFileChunk::UpdateDataSize() [this=%p, offset=%d, len=%d, EOF=%d]",
-       this, aOffset, aLen, aEOF));
+  LOG(("CacheFileChunk::UpdateDataSize() [this=%p, offset=%d, len=%d]",
+       this, aOffset, aLen));
 
   mIsDirty = true;
 
   int64_t fileSize = kChunkSize * mIndex + aOffset + aLen;
   bool notify = false;
 
-  if (fileSize > mFile->mDataSize)
+  if (fileSize > mFile->mDataSize) {
     mFile->mDataSize = fileSize;
-
-  if (aOffset + aLen > mDataSize) {
-    mDataSize = aOffset + aLen;
     notify = true;
   }
 
   if (mState == READY || mState == WRITING) {
     MOZ_ASSERT(mValidityMap.Length() == 0);
 
     if (notify) {
       NotifyUpdateListeners();
@@ -430,37 +634,23 @@ CacheFileChunk::OnDataWritten(CacheFileH
   nsCOMPtr<CacheFileChunkListener> listener;
 
   {
     CacheFileAutoLock lock(mFile);
 
     MOZ_ASSERT(mState == WRITING);
     MOZ_ASSERT(mListener);
 
+    mWritingStateHandle = nullptr;
+
     if (NS_WARN_IF(NS_FAILED(aResult))) {
       SetError(aResult);
     }
 
     mState = READY;
-
-    if (!mBuf) {
-      mBuf = mRWBuf;
-      mBufSize = mRWBufSize;
-      mRWBuf = nullptr;
-      mRWBufSize = 0;
-    } else {
-      CacheFileUtils::FreeBuffer(mRWBuf);
-      mRWBuf = nullptr;
-      mRWBufSize = 0;
-      ChunkAllocationChanged();
-    }
-
-
-    DoMemoryReport(MemorySize());
-
     mListener.swap(listener);
   }
 
   listener->OnChunkWritten(aResult, this);
 
   return NS_OK;
 }
 
@@ -473,98 +663,50 @@ CacheFileChunk::OnDataRead(CacheFileHand
 
   nsCOMPtr<CacheFileChunkListener> listener;
 
   {
     CacheFileAutoLock lock(mFile);
 
     MOZ_ASSERT(mState == READING);
     MOZ_ASSERT(mListener);
+    MOZ_ASSERT(mReadingStateBuf);
+    MOZ_RELEASE_ASSERT(mBuf->ReadHandlesCount() == 0);
+    MOZ_RELEASE_ASSERT(!mBuf->WriteHandleExists());
+
+    RefPtr<CacheFileChunkBuffer> tmpBuf;
+    tmpBuf.swap(mReadingStateBuf);
 
     if (NS_SUCCEEDED(aResult)) {
-      CacheHash::Hash16_t hash = CacheHash::Hash16(mRWBuf, mRWBufSize);
-      if (hash != mReadHash) {
+      CacheHash::Hash16_t hash = CacheHash::Hash16(tmpBuf->Buf(),
+                                                   tmpBuf->DataSize());
+      if (hash != mExpectedHash) {
         LOG(("CacheFileChunk::OnDataRead() - Hash mismatch! Hash of the data is"
              " %hx, hash in metadata is %hx. [this=%p, idx=%d]",
-             hash, mReadHash, this, mIndex));
+             hash, mExpectedHash, this, mIndex));
         aResult = NS_ERROR_FILE_CORRUPTED;
-      }
-      else {
-        if (!mBuf) {
-          // Just swap the buffers if we don't have mBuf yet
-          MOZ_ASSERT(mDataSize == mRWBufSize);
-          mBuf = mRWBuf;
-          mBufSize = mRWBufSize;
-          mRWBuf = nullptr;
-          mRWBufSize = 0;
+      } else {
+        if (!mBuf->Buf()) {
+          // Just swap the buffers if mBuf is still empty
+          mBuf.swap(tmpBuf);
         } else {
           LOG(("CacheFileChunk::OnDataRead() - Merging buffers. [this=%p]",
                this));
 
-          // Merge data with write buffer
-          if (mRWBufSize >= mBufSize) {
-            // The new data will fit into the buffer that contains data read
-            // from the disk. Simply copy the valid pieces.
-            mValidityMap.Log();
-            for (uint32_t i = 0; i < mValidityMap.Length(); i++) {
-              if (mValidityMap[i].Offset() + mValidityMap[i].Len() > mBufSize) {
-                MOZ_CRASH("Unexpected error in validity map!");
-              }
-              memcpy(mRWBuf + mValidityMap[i].Offset(),
-                     mBuf + mValidityMap[i].Offset(), mValidityMap[i].Len());
-            }
-            mValidityMap.Clear();
-
-            CacheFileUtils::FreeBuffer(mBuf);
-            mBuf = mRWBuf;
-            mBufSize = mRWBufSize;
-            mRWBuf = nullptr;
-            mRWBufSize = 0;
-            ChunkAllocationChanged();
-          } else {
-            // Buffer holding the new data is larger. Use it as the destination
-            // buffer to avoid reallocating mRWBuf. We need to copy those pieces
-            // from mRWBuf which are not valid in mBuf.
-            uint32_t invalidOffset = 0;
-            uint32_t invalidLength;
-            mValidityMap.Log();
-            for (uint32_t i = 0; i < mValidityMap.Length(); i++) {
-              MOZ_ASSERT(invalidOffset <= mValidityMap[i].Offset());
-              invalidLength = mValidityMap[i].Offset() - invalidOffset;
-              if (invalidLength > 0) {
-                if (invalidOffset + invalidLength > mRWBufSize) {
-                  MOZ_CRASH("Unexpected error in validity map!");
-                }
-                memcpy(mBuf + invalidOffset, mRWBuf + invalidOffset,
-                       invalidLength);
-              }
-              invalidOffset = mValidityMap[i].Offset() + mValidityMap[i].Len();
-            }
-            if (invalidOffset < mRWBufSize) {
-              invalidLength = mRWBufSize - invalidOffset;
-              memcpy(mBuf + invalidOffset, mRWBuf + invalidOffset,
-                     invalidLength);
-            }
-            mValidityMap.Clear();
-
-            CacheFileUtils::FreeBuffer(mRWBuf);
-            mRWBuf = nullptr;
-            mRWBufSize = 0;
-            ChunkAllocationChanged();
-          }
-
-          DoMemoryReport(MemorySize());
+          mValidityMap.Log();
+          aResult = mBuf->FillInvalidRanges(tmpBuf, &mValidityMap);
+          mValidityMap.Clear();
         }
       }
     }
 
     if (NS_FAILED(aResult)) {
       aResult = mIndex ? NS_ERROR_FILE_CORRUPTED : NS_ERROR_FILE_NOT_FOUND;
       SetError(aResult);
-      mDataSize = 0;
+      mBuf->SetDataSize(0);
     }
 
     mState = READY;
     mListener.swap(listener);
   }
 
   listener->OnChunkRead(aResult, this);
 
@@ -596,156 +738,137 @@ bool
 CacheFileChunk::IsKilled()
 {
   return mFile->IsKilled();
 }
 
 bool
 CacheFileChunk::IsReady() const
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   return (NS_SUCCEEDED(mStatus) && (mState == READY || mState == WRITING));
 }
 
 bool
 CacheFileChunk::IsDirty() const
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   return mIsDirty;
 }
 
 nsresult
 CacheFileChunk::GetStatus()
 {
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
   return mStatus;
 }
 
 void
 CacheFileChunk::SetError(nsresult aStatus)
 {
+  LOG(("CacheFileChunk::SetError() [this=%p, status=0x%08x]", this, aStatus));
+
   MOZ_ASSERT(NS_FAILED(aStatus));
 
   if (NS_FAILED(mStatus)) {
     // Remember only the first error code.
     return;
   }
 
   mStatus = aStatus;
 }
 
-char *
-CacheFileChunk::BufForWriting() const
+CacheFileChunkReadHandle
+CacheFileChunk::GetReadHandle()
 {
-  mFile->AssertOwnsLock();
+  LOG(("CacheFileChunk::GetReadHandle() [this=%p]", this));
 
-  MOZ_ASSERT(mBuf); // Writer should always first call EnsureBufSize()
+  AssertOwnsLock();
 
-  MOZ_ASSERT(NS_SUCCEEDED(mStatus));
-  MOZ_ASSERT((mState == READY && !mRWBuf) ||
-             (mState == WRITING && mRWBuf) ||
-             (mState == READING && mRWBuf));
+  MOZ_RELEASE_ASSERT(mState == READY || mState == WRITING);
+  // We don't release the lock when writing the data and CacheFileOutputStream
+  // doesn't get the read handle, so there cannot be a write handle when read
+  // handle is obtained.
+  MOZ_RELEASE_ASSERT(!mBuf->WriteHandleExists());
 
-  return mBuf;
+  return CacheFileChunkReadHandle(mBuf);
 }
 
-const char *
-CacheFileChunk::BufForReading() const
+CacheFileChunkWriteHandle
+CacheFileChunk::GetWriteHandle(uint32_t aEnsuredBufSize)
 {
-  mFile->AssertOwnsLock();
-
-  MOZ_ASSERT((mState == READY && mBuf && !mRWBuf) ||
-             (mState == WRITING && mRWBuf));
-
-  return mBuf ? mBuf : mRWBuf;
-}
+  LOG(("CacheFileChunk::GetWriteHandle() [this=%p, ensuredBufSize=%u]",
+       this, aEnsuredBufSize));
 
-MOZ_MUST_USE nsresult
-CacheFileChunk::EnsureBufSize(uint32_t aBufSize)
-{
-  mFile->AssertOwnsLock();
+  AssertOwnsLock();
 
-  // EnsureBufSize() is called only when we want to write some data to the chunk
-  // and we never write data anymore once some error occurs.
-  MOZ_ASSERT(NS_SUCCEEDED(mStatus));
-
-  if (mBufSize >= aBufSize) {
-    return NS_OK;
+  if (NS_FAILED(mStatus)) {
+    return CacheFileChunkWriteHandle(nullptr); // dummy handle
   }
 
-  bool copy = false;
-  if (!mBuf && mState == WRITING) {
-    // We need to duplicate the data that is being written on the background
-    // thread, so make sure that all the data fits into the new buffer.
-    copy = true;
+  nsresult rv;
+
+  // We don't support multiple write handles
+  MOZ_RELEASE_ASSERT(!mBuf->WriteHandleExists());
+
+  if (mBuf->ReadHandlesCount()) {
+    LOG(("CacheFileChunk::GetWriteHandle() - cloning buffer because of existing"
+         " read handle"));
 
-    if (mRWBufSize > aBufSize)
-      aBufSize = mRWBufSize;
+    MOZ_RELEASE_ASSERT(mState != READING);
+    RefPtr<CacheFileChunkBuffer> newBuf = new CacheFileChunkBuffer(this);
+    rv = newBuf->EnsureBufSize(std::max(aEnsuredBufSize, mBuf->DataSize()));
+    if (NS_SUCCEEDED(rv)) {
+      newBuf->CopyFrom(mBuf);
+      mOldBufs.AppendElement(mBuf);
+      mBuf = newBuf;
+    }
+  } else {
+    rv = mBuf->EnsureBufSize(aEnsuredBufSize);
   }
 
-  // find smallest power of 2 greater than or equal to aBufSize
-  aBufSize--;
-  aBufSize |= aBufSize >> 1;
-  aBufSize |= aBufSize >> 2;
-  aBufSize |= aBufSize >> 4;
-  aBufSize |= aBufSize >> 8;
-  aBufSize |= aBufSize >> 16;
-  aBufSize++;
-
-  const uint32_t minBufSize = kMinBufSize;
-  const uint32_t maxBufSize = kChunkSize;
-  aBufSize = clamped(aBufSize, minBufSize, maxBufSize);
-
-  if (!CanAllocate(aBufSize - mBufSize)) {
+  if (NS_FAILED(rv)) {
     SetError(NS_ERROR_OUT_OF_MEMORY);
-    return mStatus;
+    return CacheFileChunkWriteHandle(nullptr); // dummy handle
   }
 
-  char *newBuf = static_cast<char *>(realloc(mBuf, aBufSize));
-  if (!newBuf) {
-    SetError(NS_ERROR_OUT_OF_MEMORY);
-    return mStatus;
-  }
-
-  mBuf = newBuf;
-  mBufSize = aBufSize;
-  ChunkAllocationChanged();
-
-  if (copy)
-    memcpy(mBuf, mRWBuf, mRWBufSize);
-
-  DoMemoryReport(MemorySize());
-
-  return NS_OK;
+  return CacheFileChunkWriteHandle(mBuf);
 }
 
 // Memory reporting
 
 size_t
 CacheFileChunk::SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
-  size_t n = 0;
-  n += mallocSizeOf(mBuf);
-  n += mallocSizeOf(mRWBuf);
+  size_t n = mBuf->SizeOfIncludingThis(mallocSizeOf);
+
+  if (mReadingStateBuf) {
+    n += mReadingStateBuf->SizeOfIncludingThis(mallocSizeOf);
+  }
+
+  for (uint32_t i = 0; i < mOldBufs.Length(); ++i) {
+    n += mOldBufs[i]->SizeOfIncludingThis(mallocSizeOf);
+  }
+
   n += mValidityMap.SizeOfExcludingThis(mallocSizeOf);
 
   return n;
 }
 
 size_t
 CacheFileChunk::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
   return mallocSizeOf(this) + SizeOfExcludingThis(mallocSizeOf);
 }
 
 bool
-CacheFileChunk::CanAllocate(uint32_t aSize)
+CacheFileChunk::CanAllocate(uint32_t aSize) const
 {
   if (!mLimitAllocation) {
     return true;
   }
 
   LOG(("CacheFileChunk::CanAllocate() [this=%p, size=%u]", this, aSize));
 
   uint32_t limit = CacheObserver::MaxDiskChunksMemoryUsage(mIsPriority);
@@ -758,31 +881,36 @@ CacheFileChunk::CanAllocate(uint32_t aSi
     LOG(("CacheFileChunk::CanAllocate() - Returning false. [this=%p]", this));
     return false;
   }
 
   return true;
 }
 
 void
-CacheFileChunk::ChunkAllocationChanged()
+CacheFileChunk::BuffersAllocationChanged(uint32_t aFreed, uint32_t aAllocated)
 {
+  uint32_t oldBuffersSize = mBuffersSize;
+  mBuffersSize += aAllocated;
+  mBuffersSize -= aFreed;
+
+  DoMemoryReport(sizeof(CacheFileChunk) + mBuffersSize);
+
   if (!mLimitAllocation) {
     return;
   }
 
-  ChunksMemoryUsage() -= mReportedAllocation;
-  mReportedAllocation = mBufSize + mRWBufSize;
-  ChunksMemoryUsage() += mReportedAllocation;
-  LOG(("CacheFileChunk::ChunkAllocationChanged() - %s chunks usage %u "
+  ChunksMemoryUsage() -= oldBuffersSize;
+  ChunksMemoryUsage() += mBuffersSize;
+  LOG(("CacheFileChunk::BuffersAllocationChanged() - %s chunks usage %u "
        "[this=%p]", mIsPriority ? "Priority" : "Normal",
        static_cast<uint32_t>(ChunksMemoryUsage()), this));
 }
 
-mozilla::Atomic<uint32_t>& CacheFileChunk::ChunksMemoryUsage()
+mozilla::Atomic<uint32_t, ReleaseAcquire>& CacheFileChunk::ChunksMemoryUsage() const
 {
-  static mozilla::Atomic<uint32_t> chunksMemoryUsage(0);
-  static mozilla::Atomic<uint32_t> prioChunksMemoryUsage(0);
+  static mozilla::Atomic<uint32_t, ReleaseAcquire> chunksMemoryUsage(0);
+  static mozilla::Atomic<uint32_t, ReleaseAcquire> prioChunksMemoryUsage(0);
   return mIsPriority ? prioChunksMemoryUsage : chunksMemoryUsage;
 }
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/cache2/CacheFileChunk.h
+++ b/netwerk/cache2/CacheFileChunk.h
@@ -16,16 +16,87 @@ namespace mozilla {
 namespace net {
 
 #define kChunkSize        (256 * 1024)
 #define kEmptyChunkHash   0x1826
 
 class CacheFileChunk;
 class CacheFile;
 
+class CacheFileChunkBuffer
+{
+public:
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CacheFileChunkBuffer)
+
+  explicit CacheFileChunkBuffer(CacheFileChunk *aChunk);
+
+  nsresult EnsureBufSize(uint32_t aSize);
+  void     CopyFrom(CacheFileChunkBuffer *aOther);
+  nsresult FillInvalidRanges(CacheFileChunkBuffer *aOther,
+                             CacheFileUtils::ValidityMap *aMap);
+  size_t   SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
+
+  char *   Buf() const { return mBuf; }
+  void     SetDataSize(uint32_t aDataSize);
+  uint32_t DataSize() const { return mDataSize; }
+  uint32_t ReadHandlesCount() const { return mReadHandlesCount; }
+  bool     WriteHandleExists() const { return mWriteHandleExists; }
+
+private:
+  friend class CacheFileChunkHandle;
+  friend class CacheFileChunkReadHandle;
+  friend class CacheFileChunkWriteHandle;
+
+  ~CacheFileChunkBuffer();
+
+  void AssertOwnsLock() const;
+
+  void RemoveReadHandle();
+  void RemoveWriteHandle();
+
+  // We keep a weak reference to the chunk to not create a reference cycle. The
+  // buffer is referenced only by chunk and handles. Handles are always
+  // destroyed before the chunk so it is guaranteed that mChunk is a valid
+  // pointer for the whole buffer's lifetime.
+  CacheFileChunk *mChunk;
+  char           *mBuf;
+  uint32_t        mBufSize;
+  uint32_t        mDataSize;
+  uint32_t        mReadHandlesCount;
+  bool            mWriteHandleExists;
+};
+
+class CacheFileChunkHandle
+{
+public:
+  uint32_t DataSize();
+  uint32_t Offset();
+
+protected:
+  RefPtr<CacheFileChunkBuffer> mBuf;
+};
+
+class CacheFileChunkReadHandle : public CacheFileChunkHandle
+{
+public:
+  explicit CacheFileChunkReadHandle(CacheFileChunkBuffer *aBuf);
+  ~CacheFileChunkReadHandle();
+
+  const char *Buf();
+};
+
+class CacheFileChunkWriteHandle : public CacheFileChunkHandle
+{
+public:
+  explicit CacheFileChunkWriteHandle(CacheFileChunkBuffer *aBuf);
+  ~CacheFileChunkWriteHandle();
+
+  char *Buf();
+  void UpdateDataSize(uint32_t aOffset, uint32_t aLen);
+};
 
 #define CACHEFILECHUNKLISTENER_IID \
 { /* baf16149-2ab5-499c-a9c2-5904eb95c288 */       \
   0xbaf16149,                                      \
   0x2ab5,                                          \
   0x499c,                                          \
   {0xa9, 0xc2, 0x59, 0x04, 0xeb, 0x95, 0xc2, 0x88} \
 }
@@ -76,85 +147,98 @@ public:
   nsresult Read(CacheFileHandle *aHandle, uint32_t aLen,
                 CacheHash::Hash16_t aHash,
                 CacheFileChunkListener *aCallback);
   nsresult Write(CacheFileHandle *aHandle, CacheFileChunkListener *aCallback);
   void     WaitForUpdate(CacheFileChunkListener *aCallback);
   nsresult CancelWait(CacheFileChunkListener *aCallback);
   nsresult NotifyUpdateListeners();
 
-  uint32_t            Index();
-  CacheHash::Hash16_t Hash();
-  uint32_t            DataSize();
-  void                UpdateDataSize(uint32_t aOffset, uint32_t aLen,
-                                     bool aEOF);
+  uint32_t            Index() const;
+  CacheHash::Hash16_t Hash() const;
+  uint32_t            DataSize() const;
 
   NS_IMETHOD OnFileOpened(CacheFileHandle *aHandle, nsresult aResult) override;
   NS_IMETHOD OnDataWritten(CacheFileHandle *aHandle, const char *aBuf,
                            nsresult aResult) override;
   NS_IMETHOD OnDataRead(CacheFileHandle *aHandle, char *aBuf, nsresult aResult) override;
   NS_IMETHOD OnFileDoomed(CacheFileHandle *aHandle, nsresult aResult) override;
   NS_IMETHOD OnEOFSet(CacheFileHandle *aHandle, nsresult aResult) override;
   NS_IMETHOD OnFileRenamed(CacheFileHandle *aHandle, nsresult aResult) override;
   virtual bool IsKilled() override;
 
   bool   IsReady() const;
   bool   IsDirty() const;
 
   nsresult GetStatus();
   void     SetError(nsresult aStatus);
 
-  char *       BufForWriting() const;
-  const char * BufForReading() const;
-  nsresult     EnsureBufSize(uint32_t aBufSize);
-  uint32_t     MemorySize() const { return sizeof(CacheFileChunk) + mRWBufSize + mBufSize; }
+  CacheFileChunkReadHandle  GetReadHandle();
+  CacheFileChunkWriteHandle GetWriteHandle(uint32_t aEnsuredBufSize);
 
   // Memory reporting
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 private:
+  friend class CacheFileChunkBuffer;
+  friend class CacheFileChunkWriteHandle;
   friend class CacheFileInputStream;
   friend class CacheFileOutputStream;
   friend class CacheFile;
 
   virtual ~CacheFileChunk();
 
-  bool CanAllocate(uint32_t aSize);
-  void ChunkAllocationChanged();
-  mozilla::Atomic<uint32_t>& ChunksMemoryUsage();
+  void AssertOwnsLock() const;
+
+  void UpdateDataSize(uint32_t aOffset, uint32_t aLen);
+
+  bool CanAllocate(uint32_t aSize) const;
+  void BuffersAllocationChanged(uint32_t aFreed, uint32_t aAllocated);
+
+  mozilla::Atomic<uint32_t, ReleaseAcquire>& ChunksMemoryUsage() const;
 
   enum EState {
     INITIAL = 0,
     READING = 1,
     WRITING = 2,
     READY   = 3
   };
 
   uint32_t mIndex;
   EState   mState;
   nsresult mStatus;
   bool     mIsDirty;
   bool     mActiveChunk; // Is true iff the chunk is in CacheFile::mChunks.
                          // Adding/removing chunk to/from mChunks as well as
                          // changing this member happens under the CacheFile's
                          // lock.
-  uint32_t mDataSize;
 
-  uint32_t   mReportedAllocation;
+  uint32_t   mBuffersSize;
   bool const mLimitAllocation : 1; // Whether this chunk respects limit for disk
                                    // chunks memory usage.
   bool const mIsPriority : 1;
 
-  char    *mBuf;
-  uint32_t mBufSize;
+  // Buffer containing the chunk data. Multiple read handles can access the same
+  // buffer. When write handle is created and some read handle exists a new copy
+  // of the buffer is created. This prevents invalidating the buffer when
+  // CacheFileInputStream::ReadSegments calls the handler outside the lock.
+  RefPtr<CacheFileChunkBuffer> mBuf;
+
+  // We need to keep pointers of the old buffers for memory reporting.
+  nsTArray<RefPtr<CacheFileChunkBuffer>> mOldBufs;
 
-  char               *mRWBuf;
-  uint32_t            mRWBufSize;
-  CacheHash::Hash16_t mReadHash;
+  // Read handle that is used during writing the chunk to the disk.
+  nsAutoPtr<CacheFileChunkReadHandle> mWritingStateHandle;
+
+  // Buffer that is used to read the chunk from the disk. It is allowed to write
+  // a new data to chunk while we wait for the data from the disk. In this case
+  // this buffer is merged with mBuf in OnDataRead().
+  RefPtr<CacheFileChunkBuffer> mReadingStateBuf;
+  CacheHash::Hash16_t          mExpectedHash;
 
   RefPtr<CacheFile>                mFile; // is null if chunk is cached to
                                           // prevent reference cycles
   nsCOMPtr<CacheFileChunkListener> mListener;
   nsTArray<ChunkListenerItem *>    mUpdateListeners;
   CacheFileUtils::ValidityMap      mValidityMap;
 };
 
--- a/netwerk/cache2/CacheFileInputStream.cpp
+++ b/netwerk/cache2/CacheFileInputStream.cpp
@@ -40,31 +40,33 @@ NS_INTERFACE_MAP_BEGIN(CacheFileInputStr
   NS_INTERFACE_MAP_ENTRY(nsISeekableStream)
   NS_INTERFACE_MAP_ENTRY(mozilla::net::CacheFileChunkListener)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIInputStream)
 NS_INTERFACE_MAP_END_THREADSAFE
 
 CacheFileInputStream::CacheFileInputStream(CacheFile *aFile, nsISupports *aEntry)
   : mFile(aFile)
   , mPos(0)
+  , mStatus(NS_OK)
   , mClosed(false)
-  , mStatus(NS_OK)
+  , mInReadSegments(false)
   , mWaitingForUpdate(false)
   , mListeningForChunk(-1)
   , mCallbackFlags(0)
   , mCacheEntryHandle(aEntry)
 {
   LOG(("CacheFileInputStream::CacheFileInputStream() [this=%p]", this));
   MOZ_COUNT_CTOR(CacheFileInputStream);
 }
 
 CacheFileInputStream::~CacheFileInputStream()
 {
   LOG(("CacheFileInputStream::~CacheFileInputStream() [this=%p]", this));
   MOZ_COUNT_DTOR(CacheFileInputStream);
+  MOZ_ASSERT(!mInReadSegments);
 }
 
 // nsIInputStream
 NS_IMETHODIMP
 CacheFileInputStream::Close()
 {
   LOG(("CacheFileInputStream::Close() [this=%p]", this));
   return CloseWithStatus(NS_OK);
@@ -118,16 +120,22 @@ CacheFileInputStream::ReadSegments(nsWri
 
   LOG(("CacheFileInputStream::ReadSegments() [this=%p, count=%d]",
        this, aCount));
 
   nsresult rv;
 
   *_retval = 0;
 
+  if (mInReadSegments) {
+    LOG(("CacheFileInputStream::ReadSegments() - Cannot be called while the "
+         "stream is in ReadSegments!"));
+    return NS_ERROR_UNEXPECTED;
+  }
+
   if (mClosed) {
     LOG(("CacheFileInputStream::ReadSegments() - Stream is closed. [this=%p, "
          "status=0x%08x]", this, mStatus));
 
     if NS_FAILED(mStatus)
       return mStatus;
 
     return NS_OK;
@@ -137,61 +145,78 @@ CacheFileInputStream::ReadSegments(nsWri
 
   while (true) {
     if (NS_FAILED(mStatus))
       return mStatus;
 
     if (!mChunk) {
       if (mListeningForChunk == -1) {
         return NS_OK;
-      }
-      else {
+      } else {
         return NS_BASE_STREAM_WOULD_BLOCK;
       }
     }
 
-    int64_t canRead;
-    const char *buf;
-    CanRead(&canRead, &buf);
+    CacheFileChunkReadHandle hnd = mChunk->GetReadHandle();
+    int64_t canRead = CanRead(&hnd);
     if (NS_FAILED(mStatus)) {
       return mStatus;
     }
 
     if (canRead < 0) {
       // file was truncated ???
       MOZ_ASSERT(false, "SetEOF is currenty not implemented?!");
       rv = NS_OK;
     } else if (canRead > 0) {
       uint32_t toRead = std::min(static_cast<uint32_t>(canRead), aCount);
+      uint32_t read;
+      const char *buf = hnd.Buf() + (mPos - hnd.Offset());
 
-      uint32_t read;
+      mInReadSegments = true;
+      lock.Unlock();
+
       rv = aWriter(this, aClosure, buf, *_retval, toRead, &read);
 
+      lock.Lock();
+      mInReadSegments = false;
+
       if (NS_SUCCEEDED(rv)) {
         MOZ_ASSERT(read <= toRead,
                    "writer should not write more than we asked it to write");
 
         *_retval += read;
         mPos += read;
         aCount -= read;
 
-        // The last chunk is released after the caller closes this stream.
-        EnsureCorrectChunk(false);
+        if (!mClosed) {
+          if (hnd.DataSize() != mChunk->DataSize()) {
+            // New data was written to this chunk while the lock was released.
+            continue;
+          }
+
+          // The last chunk is released after the caller closes this stream.
+          EnsureCorrectChunk(false);
 
-        if (mChunk && aCount) {
-          // We have the next chunk! Go on.
-          continue;
+          if (mChunk && aCount) {
+            // We have the next chunk! Go on.
+            continue;
+          }
         }
       }
 
+      if (mClosed) {
+        // The stream was closed from aWriter, do the cleanup.
+        CleanUp();
+      }
+
       rv = NS_OK;
     } else {
-      if (mFile->mOutput)
+      if (mFile->mOutput) {
         rv = NS_BASE_STREAM_WOULD_BLOCK;
-      else {
+      } else {
         rv = NS_OK;
       }
     }
 
     break;
   }
 
   LOG(("CacheFileInputStream::ReadSegments() [this=%p, rv=0x%08x, retval=%d]",
@@ -221,48 +246,72 @@ CacheFileInputStream::CloseWithStatus(ns
 
 nsresult
 CacheFileInputStream::CloseWithStatusLocked(nsresult aStatus)
 {
   LOG(("CacheFileInputStream::CloseWithStatusLocked() [this=%p, "
        "aStatus=0x%08x]", this, aStatus));
 
   if (mClosed) {
-    MOZ_ASSERT(!mCallback);
+    // We notify listener and null out mCallback immediately after closing
+    // the stream. If we're in ReadSegments we postpone notification until we
+    // step out from ReadSegments. So if the stream is already closed the
+    // following assertion must be true.
+    MOZ_ASSERT(!mCallback || mInReadSegments);
+
     return NS_OK;
   }
 
   mClosed = true;
   mStatus = NS_FAILED(aStatus) ? aStatus : NS_BASE_STREAM_CLOSED;
 
+  if (!mInReadSegments) {
+    CleanUp();
+  }
+
+  return NS_OK;
+}
+
+void
+CacheFileInputStream::CleanUp()
+{
+  MOZ_ASSERT(!mInReadSegments);
+  MOZ_ASSERT(mClosed);
+
   if (mChunk) {
     ReleaseChunk();
   }
 
   // TODO propagate error from input stream to other streams ???
 
   MaybeNotifyListener();
 
   mFile->ReleaseOutsideLock(mCacheEntryHandle.forget());
-
-  return NS_OK;
 }
 
 NS_IMETHODIMP
 CacheFileInputStream::AsyncWait(nsIInputStreamCallback *aCallback,
                                 uint32_t aFlags,
                                 uint32_t aRequestedCount,
                                 nsIEventTarget *aEventTarget)
 {
   CacheFileAutoLock lock(mFile);
 
   LOG(("CacheFileInputStream::AsyncWait() [this=%p, callback=%p, flags=%d, "
        "requestedCount=%d, eventTarget=%p]", this, aCallback, aFlags,
        aRequestedCount, aEventTarget));
 
+  if (mInReadSegments) {
+    LOG(("CacheFileInputStream::AsyncWait() - Cannot be called while the stream"
+         " is in ReadSegments!"));
+    MOZ_ASSERT(false, "Unexpected call. If it's a valid usage implement it. "
+               "Otherwise fix the caller.");
+    return NS_ERROR_UNEXPECTED;
+  }
+
   mCallback = aCallback;
   mCallbackFlags = aFlags;
   mCallbackTarget = aEventTarget;
 
   if (!mCallback) {
     if (mWaitingForUpdate) {
       mChunk->CancelWait(this);
       mWaitingForUpdate = false;
@@ -286,16 +335,22 @@ CacheFileInputStream::AsyncWait(nsIInput
 NS_IMETHODIMP
 CacheFileInputStream::Seek(int32_t whence, int64_t offset)
 {
   CacheFileAutoLock lock(mFile);
 
   LOG(("CacheFileInputStream::Seek() [this=%p, whence=%d, offset=%lld]",
        this, whence, offset));
 
+  if (mInReadSegments) {
+    LOG(("CacheFileInputStream::Seek() - Cannot be called while the stream is "
+         "in ReadSegments!"));
+    return NS_ERROR_UNEXPECTED;
+  }
+
   if (mClosed) {
     LOG(("CacheFileInputStream::Seek() - Stream is closed. [this=%p]", this));
     return NS_BASE_STREAM_CLOSED;
   }
 
   int64_t newPos = offset;
   switch (whence) {
     case NS_SEEK_SET:
@@ -372,16 +427,17 @@ CacheFileInputStream::OnChunkAvailable(n
          "different chunk. [this=%p, listeningForChunk=%lld]",
          this, mListeningForChunk));
 
     return NS_OK;
   }
 
   MOZ_ASSERT(!mChunk);
   MOZ_ASSERT(!mWaitingForUpdate);
+  MOZ_ASSERT(!mInReadSegments);
   mListeningForChunk = -1;
 
   if (mClosed) {
     MOZ_ASSERT(!mCallback);
 
     LOG(("CacheFileInputStream::OnChunkAvailable() - Stream is closed, "
          "ignoring notification. [this=%p]", this));
 
@@ -434,16 +490,18 @@ CacheFileInputStream::OnChunkUpdated(Cac
 void
 CacheFileInputStream::ReleaseChunk()
 {
   mFile->AssertOwnsLock();
 
   LOG(("CacheFileInputStream::ReleaseChunk() [this=%p, idx=%d]",
        this, mChunk->Index()));
 
+  MOZ_ASSERT(!mInReadSegments);
+
   if (mWaitingForUpdate) {
     LOG(("CacheFileInputStream::ReleaseChunk() - Canceling waiting for update. "
          "[this=%p]", this));
 
     mChunk->CancelWait(this);
     mWaitingForUpdate = false;
   }
 
@@ -457,25 +515,31 @@ CacheFileInputStream::EnsureCorrectChunk
 
   LOG(("CacheFileInputStream::EnsureCorrectChunk() [this=%p, releaseOnly=%d]",
        this, aReleaseOnly));
 
   nsresult rv;
 
   uint32_t chunkIdx = mPos / kChunkSize;
 
+  if (mInReadSegments) {
+    // We must have correct chunk
+    MOZ_ASSERT(mChunk);
+    MOZ_ASSERT(mChunk->Index() == chunkIdx);
+    return;
+  }
+
   if (mChunk) {
     if (mChunk->Index() == chunkIdx) {
       // we have a correct chunk
       LOG(("CacheFileInputStream::EnsureCorrectChunk() - Have correct chunk "
            "[this=%p, idx=%d]", this, chunkIdx));
 
       return;
-    }
-    else {
+    } else {
       ReleaseChunk();
     }
   }
 
   MOZ_ASSERT(!mWaitingForUpdate);
 
   if (aReleaseOnly)
     return;
@@ -505,47 +569,44 @@ CacheFileInputStream::EnsureCorrectChunk
     }
   } else if (!mChunk) {
     mListeningForChunk = static_cast<int64_t>(chunkIdx);
   }
 
   MaybeNotifyListener();
 }
 
-void
-CacheFileInputStream::CanRead(int64_t *aCanRead, const char **aBuf)
+int64_t
+CacheFileInputStream::CanRead(CacheFileChunkReadHandle *aHandle)
 {
   mFile->AssertOwnsLock();
 
   MOZ_ASSERT(mChunk);
   MOZ_ASSERT(mPos / kChunkSize == mChunk->Index());
 
-  uint32_t chunkOffset = mPos - (mPos / kChunkSize) * kChunkSize;
-  *aCanRead = mChunk->DataSize() - chunkOffset;
-  if (*aCanRead > 0) {
-    *aBuf = mChunk->BufForReading() + chunkOffset;
-  } else {
-    *aBuf = nullptr;
-    if (NS_FAILED(mChunk->GetStatus())) {
-      CloseWithStatusLocked(mChunk->GetStatus());
-    }
+  int64_t retval = aHandle->Offset() + aHandle->DataSize() - mPos;
+  if (retval <= 0 && NS_FAILED(mChunk->GetStatus())) {
+    CloseWithStatusLocked(mChunk->GetStatus());
   }
 
   LOG(("CacheFileInputStream::CanRead() [this=%p, canRead=%lld]",
-       this, *aCanRead));
+       this, retval));
+
+  return retval;
 }
 
 void
 CacheFileInputStream::NotifyListener()
 {
   mFile->AssertOwnsLock();
 
   LOG(("CacheFileInputStream::NotifyListener() [this=%p]", this));
 
   MOZ_ASSERT(mCallback);
+  MOZ_ASSERT(!mInReadSegments);
 
   if (!mCallbackTarget) {
     mCallbackTarget = CacheFileIOManager::IOTarget();
     if (!mCallbackTarget) {
       LOG(("CacheFileInputStream::NotifyListener() - Cannot get Cache I/O "
            "thread! Using main thread for callback."));
       mCallbackTarget = do_GetMainThread();
     }
@@ -565,16 +626,18 @@ CacheFileInputStream::MaybeNotifyListene
 {
   mFile->AssertOwnsLock();
 
   LOG(("CacheFileInputStream::MaybeNotifyListener() [this=%p, mCallback=%p, "
        "mClosed=%d, mStatus=0x%08x, mChunk=%p, mListeningForChunk=%lld, "
        "mWaitingForUpdate=%d]", this, mCallback.get(), mClosed, mStatus,
        mChunk.get(), mListeningForChunk, mWaitingForUpdate));
 
+  MOZ_ASSERT(!mInReadSegments);
+
   if (!mCallback)
     return;
 
   if (mClosed || NS_FAILED(mStatus)) {
     NotifyListener();
     return;
   }
 
@@ -586,19 +649,18 @@ CacheFileInputStream::MaybeNotifyListene
     return;
   }
 
   MOZ_ASSERT(mPos / kChunkSize == mChunk->Index());
 
   if (mWaitingForUpdate)
     return;
 
-  int64_t canRead;
-  const char *buf;
-  CanRead(&canRead, &buf);
+  CacheFileChunkReadHandle hnd = mChunk->GetReadHandle();
+  int64_t canRead = CanRead(&hnd);
   if (NS_FAILED(mStatus)) {
     // CanRead() called CloseWithStatusLocked() which called
     // MaybeNotifyListener() so the listener was already notified. Stop here.
     MOZ_ASSERT(!mCallback);
     return;
   }
 
   if (canRead > 0) {
--- a/netwerk/cache2/CacheFileInputStream.h
+++ b/netwerk/cache2/CacheFileInputStream.h
@@ -38,32 +38,34 @@ public:
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
   uint32_t GetPosition() const { return mPos; };
 
 private:
   virtual ~CacheFileInputStream();
 
   nsresult CloseWithStatusLocked(nsresult aStatus);
+  void CleanUp();
   void ReleaseChunk();
   void EnsureCorrectChunk(bool aReleaseOnly);
 
   // CanRead returns negative value when output stream truncates the data before
   // the input stream's mPos.
-  void CanRead(int64_t *aCanRead, const char **aBuf);
+  int64_t CanRead(CacheFileChunkReadHandle *aHandle);
   void NotifyListener();
   void MaybeNotifyListener();
 
-  RefPtr<CacheFile>        mFile;
+  RefPtr<CacheFile>      mFile;
   RefPtr<CacheFileChunk> mChunk;
-  int64_t                  mPos;
-  bool                     mClosed;
-  nsresult                 mStatus;
-  bool                     mWaitingForUpdate;
-  int64_t                  mListeningForChunk;
+  int64_t                mPos;
+  nsresult               mStatus;
+  bool                   mClosed : 1;
+  bool                   mInReadSegments : 1;
+  bool                   mWaitingForUpdate : 1;
+  int64_t                mListeningForChunk;
 
   nsCOMPtr<nsIInputStreamCallback> mCallback;
   uint32_t                         mCallbackFlags;
   nsCOMPtr<nsIEventTarget>         mCallbackTarget;
   // Held purely for referencing purposes
   RefPtr<nsISupports>              mCacheEntryHandle;
 };
 
--- a/netwerk/cache2/CacheFileOutputStream.cpp
+++ b/netwerk/cache2/CacheFileOutputStream.cpp
@@ -126,28 +126,29 @@ CacheFileOutputStream::Write(const char 
     FillHole();
     if (NS_FAILED(mStatus)) {
       return mStatus;
     }
 
     uint32_t chunkOffset = mPos - (mPos / kChunkSize) * kChunkSize;
     uint32_t canWrite = kChunkSize - chunkOffset;
     uint32_t thisWrite = std::min(static_cast<uint32_t>(canWrite), aCount);
-    nsresult rv = mChunk->EnsureBufSize(chunkOffset + thisWrite);
-    if (NS_FAILED(rv)) {
-      CloseWithStatusLocked(rv);
-      return rv;
+
+    CacheFileChunkWriteHandle hnd = mChunk->GetWriteHandle(chunkOffset + thisWrite);
+    if (!hnd.Buf()) {
+      CloseWithStatusLocked(NS_ERROR_OUT_OF_MEMORY);
+      return NS_ERROR_OUT_OF_MEMORY;
     }
-    memcpy(mChunk->BufForWriting() + chunkOffset, aBuf, thisWrite);
+
+    memcpy(hnd.Buf() + chunkOffset, aBuf, thisWrite);
+    hnd.UpdateDataSize(chunkOffset, thisWrite);
 
     mPos += thisWrite;
     aBuf += thisWrite;
     aCount -= thisWrite;
-
-    mChunk->UpdateDataSize(chunkOffset, thisWrite, false);
   }
 
   EnsureCorrectChunk(true);
 
   LOG(("CacheFileOutputStream::Write() - Wrote %d bytes [this=%p]",
        *_retval, this));
 
   return NS_OK;
@@ -405,26 +406,25 @@ CacheFileOutputStream::FillHole()
 
   uint32_t pos = mPos - (mPos / kChunkSize) * kChunkSize;
   if (mChunk->DataSize() >= pos)
     return;
 
   LOG(("CacheFileOutputStream::FillHole() - Zeroing hole in chunk %d, range "
        "%d-%d [this=%p]", mChunk->Index(), mChunk->DataSize(), pos - 1, this));
 
-  nsresult rv = mChunk->EnsureBufSize(pos);
-  if (NS_FAILED(rv)) {
-    CloseWithStatusLocked(rv);
+  CacheFileChunkWriteHandle hnd = mChunk->GetWriteHandle(pos);
+  if (!hnd.Buf()) {
+    CloseWithStatusLocked(NS_ERROR_OUT_OF_MEMORY);
     return;
   }
 
-  memset(mChunk->BufForWriting() + mChunk->DataSize(), 0,
-         pos - mChunk->DataSize());
-
-  mChunk->UpdateDataSize(mChunk->DataSize(), pos - mChunk->DataSize(), false);
+  uint32_t offset = hnd.DataSize();
+  memset(hnd.Buf() + offset, 0, pos - offset);
+  hnd.UpdateDataSize(offset, pos - offset);
 }
 
 void
 CacheFileOutputStream::NotifyListener()
 {
   mFile->AssertOwnsLock();
 
   LOG(("CacheFileOutputStream::NotifyListener() [this=%p]", this));
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_bug1279246.js
@@ -0,0 +1,97 @@
+Cu.import("resource://testing-common/httpd.js");
+Cu.import("resource://gre/modules/NetUtil.jsm");
+
+var httpserver = new HttpServer();
+var pass = 0;
+var responseBody = [0x0B, 0x02, 0x80, 0x74, 0x65, 0x73, 0x74, 0x0A, 0x03];
+var responseLen = 5;
+var testUrl = "/test/brotli";
+
+
+function setupChannel() {
+    return NetUtil.newChannel({
+        uri: "http://localhost:" + httpserver.identity.primaryPort + testUrl,
+        loadUsingSystemPrincipal: true
+    });
+}
+
+function Listener() {}
+
+Listener.prototype = {
+  _buffer: null,
+
+  QueryInterface: function(iid) {
+    if (iid.equals(Components.interfaces.nsIStreamListener) ||
+        iid.equals(Components.interfaces.nsIRequestObserver) ||
+        iid.equals(Components.interfaces.nsISupports))
+      return this;
+    throw Components.results.NS_ERROR_NO_INTERFACE;
+  },
+
+  onStartRequest: function(request, ctx) {
+    do_check_eq(request.status, Cr.NS_OK);
+    this._buffer = "";
+  },
+
+  onDataAvailable: function(request, cx, stream, offset, cnt) {
+    if (pass == 0) {
+      this._buffer = this._buffer.concat(read_stream(stream, cnt));
+    } else {
+      request.QueryInterface(Ci.nsICachingChannel);
+      if (!request.isFromCache()) {
+        do_throw("Response is not from the cache");
+      }
+
+      request.cancel(Cr.NS_ERROR_ABORT);
+    }
+  },
+
+  onStopRequest: function(request, ctx, status) {
+    if (pass == 0) {
+      do_check_eq(this._buffer.length, responseLen);
+      pass++;
+
+      var channel = setupChannel();
+      channel.loadFlags = Ci.nsIRequest.VALIDATE_NEVER;
+      channel.asyncOpen2(new Listener());
+    } else {
+      httpserver.stop(do_test_finished);
+      prefs.setCharPref("network.http.accept-encoding", cePref);
+    }
+  }
+};
+
+var prefs;
+var cePref;
+function run_test() {
+  do_get_profile();
+
+  prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+  cePref = prefs.getCharPref("network.http.accept-encoding");
+  prefs.setCharPref("network.http.accept-encoding", "gzip, deflate, br");
+
+  httpserver.registerPathHandler(testUrl, handler);
+  httpserver.start(-1);
+
+  var channel = setupChannel();
+  channel.asyncOpen2(new Listener());
+
+  do_test_pending();
+}
+
+function handler(metadata, response) {
+  do_check_eq(pass, 0); // the second response must be server from the cache
+
+  response.setStatusLine(metadata.httpVersion, 200, "OK");
+  response.setHeader("Content-Type", "text/plain", false);
+  response.setHeader("Content-Encoding", "br", false);
+  response.setHeader("Content-Length", "" + responseBody.length, false);
+
+  var bos = Components.classes["@mozilla.org/binaryoutputstream;1"]
+            .createInstance(Components.interfaces.nsIBinaryOutputStream);
+  bos.setOutputStream(response.bodyOutputStream);
+
+  response.processAsync();
+  bos.writeByteArray(responseBody, responseBody.length);
+  response.finish();
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -352,8 +352,9 @@ skip-if = os == "android"
 [test_packaged_app_service_paths.js]
 [test_bug1195415.js]
 [test_cookie_blacklist.js]
 [test_getHost.js]
 [test_packaged_app_bug1214079.js]
 [test_bug412457.js]
 [test_bug464591.js]
 [test_cache-control_request.js]
+[test_bug1279246.js]