Bug 1302516 - [1.2] Add mutex locking to ZipCollection zip vector access. r=glandium
authorEugen Sawin <esawin@mozilla.com>
Mon, 12 Sep 2016 14:15:24 +0200
changeset 313921 5bfde65d691ae7b3a79214f15954f663dc7d051c
parent 313894 a8bf7dc487051faa9752137134eb66cd9c043f71
child 313922 1b8bbd5046a4fdfaf1203e0f9ec3f3e88400f229
push id30703
push usercbook@mozilla.com
push dateThu, 15 Sep 2016 10:00:06 +0000
treeherdermozilla-central@29af101880db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1302516
milestone51.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 1302516 - [1.2] Add mutex locking to ZipCollection zip vector access. r=glandium
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
@@ -501,32 +501,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,16 +1,15 @@
 /* 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 <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
@@ -178,39 +178,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());
   }