Bug 1302516 - [1.2] Add mutex locking to ZipCollection zip vector access. r=glandium, a=ritu
authorEugen Sawin <esawin@mozilla.com>
Mon, 12 Sep 2016 14:15:24 +0200
changeset 348487 2c9b14f4694542e164671187d1d9da7aa01ef118
parent 348486 87d82a4489f978b529f076ec2c26652cfbf9915e
child 348488 daf0099fc523da65acbfd3dfa03f41b08959fb91
push id6455
push userryanvm@gmail.com
push dateTue, 04 Oct 2016 21:32:31 +0000
treeherdermozilla-beta@1deb126ed5bb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium, ritu
bugs1302516
milestone50.0
Bug 1302516 - [1.2] Add mutex locking to ZipCollection zip vector access. r=glandium, a=ritu
mozglue/linker/Mappable.cpp
mozglue/linker/Mappable.h
mozglue/linker/Utils.h
mozglue/linker/Zip.cpp
--- a/mozglue/linker/Mappable.cpp
+++ b/mozglue/linker/Mappable.cpp
@@ -441,32 +441,16 @@ MappableSeekableZStream::munmap(void *ad
       return;
     }
   MOZ_CRASH("munmap called with unknown mapping");
 }
 
 void
 MappableSeekableZStream::finalize() { }
 
-class AutoLock {
-public:
-  AutoLock(pthread_mutex_t *mutex): mutex(mutex)
-  {
-    if (pthread_mutex_lock(mutex))
-      MOZ_CRASH("pthread_mutex_lock failed");
-  }
-  ~AutoLock()
-  {
-    if (pthread_mutex_unlock(mutex))
-      MOZ_CRASH("pthread_mutex_unlock failed");
-  }
-private:
-  pthread_mutex_t *mutex;
-};
-
 bool
 MappableSeekableZStream::ensure(const void *addr)
 {
   DEBUG_LOG("ensure @%p", addr);
   const void *addrPage = PageAlignedPtr(addr);
   /* Find the mapping corresponding to the given page */
   std::vector<LazyMap>::iterator map;
   for (map = lazyMaps.begin(); map < lazyMaps.end(); ++map) {
--- a/mozglue/linker/Mappable.h
+++ b/mozglue/linker/Mappable.h
@@ -1,17 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef Mappable_h
 #define Mappable_h
 
 #include <sys/types.h>
-#include <pthread.h>
 #include "Zip.h"
 #include "SeekableZStream.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/UniquePtr.h"
 #include "zlib.h"
 
 /**
  * Abstract class to handle mmap()ing from various kind of entities, such as
--- a/mozglue/linker/Utils.h
+++ b/mozglue/linker/Utils.h
@@ -1,15 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef Utils_h
 #define Utils_h
 
+#include <pthread.h>
 #include <stdint.h>
 #include <stddef.h>
 #include <sys/mman.h>
 #include <unistd.h>
 #include "mozilla/Assertions.h"
 #include "mozilla/Scoped.h"
 
 /**
@@ -592,10 +593,26 @@ void *FunctionPtr(T func)
   union {
     void *ptr;
     T func;
   } f;
   f.func = func;
   return f.ptr;
 }
 
+class AutoLock {
+public:
+  AutoLock(pthread_mutex_t *mutex): mutex(mutex)
+  {
+    if (pthread_mutex_lock(mutex))
+      MOZ_CRASH("pthread_mutex_lock failed");
+  }
+  ~AutoLock()
+  {
+    if (pthread_mutex_unlock(mutex))
+      MOZ_CRASH("pthread_mutex_unlock failed");
+  }
+private:
+  pthread_mutex_t *mutex;
+};
+
 #endif /* Utils_h */
  
--- a/mozglue/linker/Zip.cpp
+++ b/mozglue/linker/Zip.cpp
@@ -176,39 +176,47 @@ Zip::GetFirstEntry() const
   if (!entries) {
     ERROR("%s - Couldn't find central directory record", name);
   }
   return entries;
 }
 
 ZipCollection ZipCollection::Singleton;
 
+static pthread_mutex_t sZipCollectionMutex = PTHREAD_MUTEX_INITIALIZER;
+
 already_AddRefed<Zip>
 ZipCollection::GetZip(const char *path)
 {
-  /* Search the list of Zips we already have for a match */
-  for (std::vector<Zip *>::iterator it = Singleton.zips.begin();
-       it < Singleton.zips.end(); ++it) {
-    if ((*it)->GetName() && (strcmp((*it)->GetName(), path) == 0)) {
-      RefPtr<Zip> zip = *it;
-      return zip.forget();
+  {
+    AutoLock lock(&sZipCollectionMutex);
+    /* Search the list of Zips we already have for a match */
+    for (std::vector<Zip *>::iterator it = Singleton.zips.begin();
+         it < Singleton.zips.end(); ++it) {
+      if ((*it)->GetName() && (strcmp((*it)->GetName(), path) == 0)) {
+        RefPtr<Zip> zip = *it;
+        return zip.forget();
+      }
     }
   }
   return Zip::Create(path);
 }
 
 void
 ZipCollection::Register(Zip *zip)
 {
+  AutoLock lock(&sZipCollectionMutex);
+  DEBUG_LOG("ZipCollection::Register(\"%s\")", zip->GetName());
   Singleton.zips.push_back(zip);
 }
 
 void
 ZipCollection::Forget(Zip *zip)
 {
+  AutoLock lock(&sZipCollectionMutex);
   DEBUG_LOG("ZipCollection::Forget(\"%s\")", zip->GetName());
   std::vector<Zip *>::iterator it = std::find(Singleton.zips.begin(),
                                               Singleton.zips.end(), zip);
   if (*it == zip) {
     Singleton.zips.erase(it);
   } else {
     DEBUG_LOG("ZipCollection::Forget: didn't find \"%s\" in bookkeeping", zip->GetName());
   }