Bug 1035411 - Suspect lock handling in cache2. r=honzab, a=sledru
authorMichal Novotny <michal.novotny@gmail.com>
Wed, 16 Jul 2014 10:57:52 +0200
changeset 209185 741090be87848346e20b84b78b739b4d201800fe
parent 209184 f5c3bbcb3251c24dd812e3953fb04030615a1c3e
child 209186 a48cbea17696764152387d1319459fbf12f5c92f
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab, sledru
bugs1035411
milestone32.0a2
Bug 1035411 - Suspect lock handling in cache2. r=honzab, a=sledru
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;
 
@@ -157,34 +154,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;
@@ -237,18 +218,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;
 
@@ -265,17 +244,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;
 
@@ -299,17 +277,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;
   }
@@ -334,17 +311,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;
 
@@ -374,17 +350,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
@@ -425,17 +400,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
@@ -52,17 +52,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