Bug 1279246 - Hang due to CacheFileInputStream deadlock. r=honzab, a=lizzard
authorMichal Novotny <michal.novotny@gmail.com>
Tue, 12 Jul 2016 17:58:38 +0200
changeset 342377 6d31ae76180dc1ab8d826a44a54f4ce59a9e3fa8
parent 342376 c555123fe9bb99eac8a088f9198697eab1cb7f00
child 342378 771ac7000529a29861deb60a60a291917dc59ae4
push id1183
push userraliiev@mozilla.com
push dateMon, 05 Sep 2016 20:01:49 +0000
treeherdermozilla-release@3148731bed45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, lizzard
bugs1279246
milestone49.0
Bug 1279246 - Hang due to CacheFileInputStream deadlock. r=honzab, a=lizzard
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]