Bug 897723 - Allow faulty.lib's on-demand decompression to be reentrant. r=nfroyd
authorMike Hommey <mh+mozilla@glandium.org>
Fri, 26 Jul 2013 12:57:53 +0900
changeset 140127 4716f738ef0a2bdb06e5efcb00716f84f2df13eb
parent 140099 52f9e8ffe111884e934c7efeae4ffea67a44b128
child 140128 df8423fdcad845e23ee1f2a3d2f8f13c67142c1a
push id25016
push userryanvm@gmail.com
push dateSat, 27 Jul 2013 02:25:56 +0000
treeherdermozilla-central@fb48c7d58b8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs897723
milestone25.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 897723 - Allow faulty.lib's on-demand decompression to be reentrant. r=nfroyd
mozglue/linker/Mappable.cpp
mozglue/linker/Mappable.h
--- a/mozglue/linker/Mappable.cpp
+++ b/mozglue/linker/Mappable.cpp
@@ -339,17 +339,21 @@ MappableDeflate::GetLength() const
 Mappable *
 MappableSeekableZStream::Create(const char *name, Zip *zip,
                                 Zip::Stream *stream)
 {
   MOZ_ASSERT(stream->GetType() == Zip::Stream::STORE);
   mozilla::ScopedDeletePtr<MappableSeekableZStream> mappable;
   mappable = new MappableSeekableZStream(zip);
 
-  if (pthread_mutex_init(&mappable->mutex, NULL))
+  pthread_mutexattr_t recursiveAttr;
+  pthread_mutexattr_init(&recursiveAttr);
+  pthread_mutexattr_settype(&recursiveAttr, PTHREAD_MUTEX_RECURSIVE);
+
+  if (pthread_mutex_init(&mappable->mutex, &recursiveAttr))
     return NULL;
 
   if (!mappable->zStream.Init(stream->GetBuffer(), stream->GetSize()))
     return NULL;
 
   mappable->buffer = _MappableBuffer::Create(name,
                               mappable->zStream.GetUncompressedSize());
   if (!mappable->buffer)
@@ -461,16 +465,29 @@ MappableSeekableZStream::ensure(const vo
   if ((it == lazyMaps.end()) || (chunkEnd > it->endOffset())) {
     /* The mapping "it" points at now is past the interesting one */
     --it;
     length = it->endOffset() - chunkStart;
   }
 
   length = PageAlignedSize(length);
 
+  /* The following lock can be re-acquired by the thread holding it.
+   * If this happens, it means the following code is interrupted somehow by
+   * some signal, and ends up retriggering a chunk decompression for the
+   * same MappableSeekableZStream.
+   * If the chunk to decompress is different the second time, then everything
+   * is safe as the only common data touched below is chunkAvailNum, and it is
+   * atomically updated (leaving out any chance of an interruption while it is
+   * updated affecting the result). If the chunk to decompress is the same, the
+   * worst thing that can happen is chunkAvailNum being incremented one too
+   * many times, which doesn't affect functionality. The chances of it
+   * happening being pretty slim, and the effect being harmless, we can just
+   * ignore the issue. Other than that, we'd just be wasting time decompressing
+   * the same chunk twice. */
   AutoLock lock(&mutex);
 
   /* The very first page is mapped and accessed separately of the rest, and
    * as such, only the first page of the first chunk is decompressed this way.
    * When we fault in the remaining pages of that chunk, we want to decompress
    * the complete chunk again. Short of doing that, we would end up with
    * no data between PageSize() and chunkSize, which would effectively corrupt
    * symbol resolution in the underlying library. */
@@ -517,17 +534,17 @@ MappableSeekableZStream::ensure(const vo
   return false;
 }
 
 void
 MappableSeekableZStream::stats(const char *when, const char *name) const
 {
   size_t nEntries = zStream.GetChunksNum();
   DEBUG_LOG("%s: %s; %" PRIdSize "/%" PRIdSize " chunks decompressed",
-            name, when, chunkAvailNum, nEntries);
+            name, when, static_cast<size_t>(chunkAvailNum), nEntries);
 
   size_t len = 64;
   mozilla::ScopedDeleteArray<char> map;
   map = new char[len + 3];
   map[0] = '[';
 
   for (size_t i = 0, j = 1; i < nEntries; i++, j++) {
     map[j] = chunkAvail[i] ? '*' : '_';
--- a/mozglue/linker/Mappable.h
+++ b/mozglue/linker/Mappable.h
@@ -260,15 +260,15 @@ private:
   /* List of all mappings */
   std::vector<LazyMap> lazyMaps;
 
   /* Array keeping track of which chunks have already been decompressed.
    * Each value is the number of pages decompressed for the given chunk. */
   mozilla::ScopedDeleteArray<unsigned char> chunkAvail;
 
   /* Number of chunks that have already been decompressed. */
-  size_t chunkAvailNum;
+  mozilla::Atomic<size_t> chunkAvailNum;
 
   /* Mutex protecting decompression */
   pthread_mutex_t mutex;
 };
 
 #endif /* Mappable_h */