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 140134 4716f738ef0a2bdb06e5efcb00716f84f2df13eb
parent 140103 52f9e8ffe111884e934c7efeae4ffea67a44b128
child 140135 df8423fdcad845e23ee1f2a3d2f8f13c67142c1a
push id1945
push userryanvm@gmail.com
push dateSat, 27 Jul 2013 02:27:26 +0000
treeherderfx-team@4874fa438b1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs897723
milestone25.0a1
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 */