Avoid potential race condition in nsJAR.

1 files changed, 18 insertions, 8 deletions

diff --git a/modules/libjar/nsJAR.cpp b/modules/libjar/nsJAR.cpp
index c7f5ebc96..aa673ea50 100644
--- a/modules/libjar/nsJAR.cpp
+++ b/modules/libjar/nsJAR.cpp
@@ -97,11 +97,27 @@ NS_IMPL_QUERY_INTERFACE(nsJAR, nsIZipReader)
 NS_IMPL_ADDREF(nsJAR)
 
 // Custom Release method works with nsZipReaderCache...
+// Release might be called from multi-thread, we have to
+// take this function carefully to avoid delete-after-use.
 MozExternalRefCountType nsJAR::Release(void)
 {
   nsrefcnt count;
   NS_PRECONDITION(0 != mRefCnt, "dup release");
-  count = --mRefCnt;
+
+  nsRefPtr<nsZipReaderCache> cache;
+  if (mRefCnt == 2) { // don't use a lock too frequently
+    // Use a mutex here to guarantee mCache is not racing and the target instance
+    // is still valid to increase ref-count.
+    MutexAutoLock lock(mLock);
+    cache = mCache;
+    mCache = nullptr;
+  }
+  if (cache) {
+    nsresult rv = cache->ReleaseZip(this);
+    MOZ_ASSERT(NS_SUCCEEDED(rv), "failed to release zip file");
+  }
+
+  count = --mRefCnt; // don't access any member variable after this line
   NS_LOG_RELEASE(this, count, "nsJAR");
   if (0 == count) {
     mRefCnt = 1; /* stabilize */
@@ -110,13 +126,7 @@ MozExternalRefCountType nsJAR::Release(void)
     delete this;
     return 0;
   }
-  else if (1 == count && mCache) {
-#ifdef DEBUG
-    nsresult rv =
-#endif
-      mCache->ReleaseZip(this);
-    NS_ASSERTION(NS_SUCCEEDED(rv), "failed to release zip file");
-  }
+
   return count;
 }