Bug 1035411 - Suspect lock handling in cache2, r=honzab
authorMichal Novotny <michal.novotny@gmail.com>
Wed, 16 Jul 2014 10:57:52 +0200
changeset 216272 6f0a5bfa00a66e8be137eb39dbdc65210739e714
parent 216271 a078bf8b709dafe3d9337a1fdbf550950a6b7135
child 216273 1b29d99921e9d6a8726e5c4df60ef18181168eea
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs1035411
milestone33.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 1035411 - Suspect lock handling in cache2, r=honzab
netwerk/cache2/CacheFileInputStream.cpp
netwerk/cache2/CacheFileInputStream.h
--- a/netwerk/cache2/CacheFileInputStream.cpp
+++ b/netwerk/cache2/CacheFileInputStream.cpp
@@ -44,17 +44,16 @@ NS_INTERFACE_MAP_END_THREADSAFE
 
 CacheFileInputStream::CacheFileInputStream(CacheFile *aFile)
   : mFile(aFile)
   , mPos(0)
   , mClosed(false)
   , mStatus(NS_OK)
   , mWaitingForUpdate(false)
   , mListeningForChunk(-1)
-  , mInReadSegments(false)
   , mCallbackFlags(0)
 {
   LOG(("CacheFileInputStream::CacheFileInputStream() [this=%p]", this));
   MOZ_COUNT_CTOR(CacheFileInputStream);
 }
 
 CacheFileInputStream::~CacheFileInputStream()
 {
@@ -69,17 +68,16 @@ CacheFileInputStream::Close()
   LOG(("CacheFileInputStream::Close() [this=%p]", this));
   return CloseWithStatus(NS_OK);
 }
 
 NS_IMETHODIMP
 CacheFileInputStream::Available(uint64_t *_retval)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   if (mClosed) {
     LOG(("CacheFileInputStream::Available() - Stream is closed. [this=%p, "
          "status=0x%08x]", this, mStatus));
     return NS_FAILED(mStatus) ? mStatus : NS_BASE_STREAM_CLOSED;
   }
 
   EnsureCorrectChunk(false);
@@ -111,17 +109,16 @@ CacheFileInputStream::Read(char *aBuf, u
   return ReadSegments(NS_CopySegmentToBuffer, aBuf, aCount, _retval);
 }
 
 NS_IMETHODIMP
 CacheFileInputStream::ReadSegments(nsWriteSegmentFun aWriter, void *aClosure,
                                    uint32_t aCount, uint32_t *_retval)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   LOG(("CacheFileInputStream::ReadSegments() [this=%p, count=%d]",
        this, aCount));
 
   nsresult rv;
 
   *_retval = 0;
 
@@ -159,34 +156,18 @@ CacheFileInputStream::ReadSegments(nsWri
 
     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);
 
-      // We need to release the lock to avoid lock re-entering unless the
-      // caller is Read() method.
-#ifdef DEBUG
-      int64_t oldPos = mPos;
-#endif
-      mInReadSegments = true;
-      if (aWriter != (nsWriteSegmentFun)NS_CopySegmentToBuffer) {
-        lock.Unlock();
-      }
       uint32_t read;
       rv = aWriter(this, aClosure, buf, *_retval, toRead, &read);
-      if (aWriter != (nsWriteSegmentFun)NS_CopySegmentToBuffer) {
-        lock.Lock();
-      }
-      mInReadSegments = false;
-#ifdef DEBUG
-      MOZ_ASSERT(oldPos == mPos);
-#endif
 
       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;
@@ -238,18 +219,16 @@ CacheFileInputStream::CloseWithStatus(ns
 }
 
 nsresult
 CacheFileInputStream::CloseWithStatusLocked(nsresult aStatus)
 {
   LOG(("CacheFileInputStream::CloseWithStatusLocked() [this=%p, "
        "aStatus=0x%08x]", this, aStatus));
 
-  MOZ_ASSERT(!mInReadSegments);
-
   if (mClosed) {
     MOZ_ASSERT(!mCallback);
     return NS_OK;
   }
 
   mClosed = true;
   mStatus = NS_FAILED(aStatus) ? aStatus : NS_BASE_STREAM_CLOSED;
 
@@ -266,17 +245,16 @@ CacheFileInputStream::CloseWithStatusLoc
 
 NS_IMETHODIMP
 CacheFileInputStream::AsyncWait(nsIInputStreamCallback *aCallback,
                                 uint32_t aFlags,
                                 uint32_t aRequestedCount,
                                 nsIEventTarget *aEventTarget)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   LOG(("CacheFileInputStream::AsyncWait() [this=%p, callback=%p, flags=%d, "
        "requestedCount=%d, eventTarget=%p]", this, aCallback, aFlags,
        aRequestedCount, aEventTarget));
 
   mCallback = aCallback;
   mCallbackFlags = aFlags;
   mCallbackTarget = aEventTarget;
@@ -301,17 +279,16 @@ CacheFileInputStream::AsyncWait(nsIInput
   return NS_OK;
 }
 
 // nsISeekableStream
 NS_IMETHODIMP
 CacheFileInputStream::Seek(int32_t whence, int64_t offset)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   LOG(("CacheFileInputStream::Seek() [this=%p, whence=%d, offset=%lld]",
        this, whence, offset));
 
   if (mClosed) {
     LOG(("CacheFileInputStream::Seek() - Stream is closed. [this=%p]", this));
     return NS_BASE_STREAM_CLOSED;
   }
@@ -336,17 +313,16 @@ CacheFileInputStream::Seek(int32_t whenc
   LOG(("CacheFileInputStream::Seek() [this=%p, pos=%lld]", this, mPos));
   return NS_OK;
 }
 
 NS_IMETHODIMP
 CacheFileInputStream::Tell(int64_t *_retval)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   if (mClosed) {
     LOG(("CacheFileInputStream::Tell() - Stream is closed. [this=%p]", this));
     return NS_BASE_STREAM_CLOSED;
   }
 
   *_retval = mPos;
 
@@ -376,17 +352,16 @@ CacheFileInputStream::OnChunkWritten(nsr
   return NS_ERROR_UNEXPECTED;
 }
 
 nsresult
 CacheFileInputStream::OnChunkAvailable(nsresult aResult, uint32_t aChunkIdx,
                                        CacheFileChunk *aChunk)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   LOG(("CacheFileInputStream::OnChunkAvailable() [this=%p, result=0x%08x, "
        "idx=%d, chunk=%p]", this, aResult, aChunkIdx, aChunk));
 
   MOZ_ASSERT(mListeningForChunk != -1);
 
   if (mListeningForChunk != static_cast<int64_t>(aChunkIdx)) {
     // This is not a chunk that we're waiting for
@@ -427,17 +402,16 @@ CacheFileInputStream::OnChunkAvailable(n
 
   return NS_OK;
 }
 
 nsresult
 CacheFileInputStream::OnChunkUpdated(CacheFileChunk *aChunk)
 {
   CacheFileAutoLock lock(mFile);
-  MOZ_ASSERT(!mInReadSegments);
 
   LOG(("CacheFileInputStream::OnChunkUpdated() [this=%p, idx=%d]",
        this, aChunk->Index()));
 
   if (!mWaitingForUpdate) {
     LOG(("CacheFileInputStream::OnChunkUpdated() - Ignoring notification since "
          "mWaitingforUpdate == false. [this=%p]", this));
 
--- a/netwerk/cache2/CacheFileInputStream.h
+++ b/netwerk/cache2/CacheFileInputStream.h
@@ -55,17 +55,16 @@ private:
 
   nsRefPtr<CacheFile>      mFile;
   nsRefPtr<CacheFileChunk> mChunk;
   int64_t                  mPos;
   bool                     mClosed;
   nsresult                 mStatus;
   bool                     mWaitingForUpdate;
   int64_t                  mListeningForChunk;
-  bool                     mInReadSegments;
 
   nsCOMPtr<nsIInputStreamCallback> mCallback;
   uint32_t                         mCallbackFlags;
   nsCOMPtr<nsIEventTarget>         mCallbackTarget;
 };
 
 
 } // net