[poppler] CMakeLists.txt glib/CMakeLists.txt goo/GooMutex.h poppler/Annot.cc poppler/Annot.h poppler/Catalog.cc poppler/Catalog.h poppler/XRef.cc utils/CMakeLists.txt

Albert Astals Cid aacid at kemper.freedesktop.org
Sat Apr 6 08:02:53 PDT 2013


 CMakeLists.txt       |    3 +++
 glib/CMakeLists.txt  |    3 +++
 goo/GooMutex.h       |   36 ++++++++++++++++++++++--------------
 poppler/Annot.cc     |   34 +++++++++++++++++-----------------
 poppler/Annot.h      |    4 ++--
 poppler/Catalog.cc   |   24 +++++++++++-------------
 poppler/Catalog.h    |    8 ++++----
 poppler/XRef.cc      |    2 +-
 utils/CMakeLists.txt |    3 +++
 9 files changed, 66 insertions(+), 51 deletions(-)

New commits:
commit 837e3704e76ea857b3de45503840e9b855096fef
Author: Albert Astals Cid <aacid at kde.org>
Date:   Sat Apr 6 17:01:08 2013 +0200

    Make our mutexes recursive
    
    Fixes a deadlock problem found with a pdf i can't share (411klaralv.pdf)
    Reviewed by Thomas and Adam on the mailing list

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4643ae0..3caa648 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -398,6 +398,9 @@ add_library(poppler SHARED ${poppler_SRCS})
 endif(MSVC)
 set_target_properties(poppler PROPERTIES VERSION 35.0.0 SOVERSION 35)
 target_link_libraries(poppler ${poppler_LIBS})
+if(HAVE_PTHREAD)
+  target_link_libraries(poppler -lpthread)
+endif()
 target_link_libraries(poppler LINK_INTERFACE_LIBRARIES "")
 install(TARGETS poppler RUNTIME DESTINATION bin LIBRARY DESTINATION lib${LIB_SUFFIX} ARCHIVE DESTINATION lib${LIB_SUFFIX})
 
diff --git a/glib/CMakeLists.txt b/glib/CMakeLists.txt
index bab5838..a3a9d4c 100644
--- a/glib/CMakeLists.txt
+++ b/glib/CMakeLists.txt
@@ -81,6 +81,9 @@ set(poppler_glib_generated_SRCS
 add_library(poppler-glib SHARED ${poppler_glib_SRCS} ${poppler_glib_generated_SRCS})
 set_target_properties(poppler-glib PROPERTIES VERSION 8.6.0 SOVERSION 8)
 target_link_libraries(poppler-glib poppler ${GLIB2_LIBRARIES} ${CAIRO_LIBRARIES} ${FREETYPE_LIBRARIES})
+if(HAVE_PTHREAD)
+   target_link_libraries(poppler-glib -lpthread)
+endif()
 install(TARGETS poppler-glib RUNTIME DESTINATION bin LIBRARY DESTINATION lib${LIB_SUFFIX} ARCHIVE DESTINATION lib${LIB_SUFFIX})
 
 install(FILES
diff --git a/goo/GooMutex.h b/goo/GooMutex.h
index 591a7d4..4d425a2 100644
--- a/goo/GooMutex.h
+++ b/goo/GooMutex.h
@@ -53,28 +53,36 @@ typedef CRITICAL_SECTION GooMutex;
 
 #include <pthread.h>
 
-typedef pthread_mutex_t GooMutex;
-
-#define gInitMutex(m) pthread_mutex_init(m, NULL)
-#define gDestroyMutex(m) pthread_mutex_destroy(m)
-#define gLockMutex(m) pthread_mutex_lock(m)
-#define gUnlockMutex(m) pthread_mutex_unlock(m)
+typedef struct {
+  pthread_mutexattr_t attributes;
+  pthread_mutex_t mutex;
+} GooMutex;
+
+inline void gInitMutex(GooMutex *m) {
+  pthread_mutexattr_init(&m->attributes);
+  pthread_mutexattr_settype(&m->attributes, PTHREAD_MUTEX_RECURSIVE);
+  pthread_mutex_init(&m->mutex, &m->attributes);
+}
+inline void gDestroyMutex(GooMutex *m) {
+  pthread_mutex_destroy(&m->mutex);
+  pthread_mutexattr_destroy(&m->attributes);
+}
+inline void gLockMutex(GooMutex *m) {
+  pthread_mutex_lock(&m->mutex);
+}
+inline void gUnlockMutex(GooMutex *m) {
+  pthread_mutex_unlock(&m->mutex);
+}
 
 #endif
 
-enum MutexLockMode {
-  DoNotLockMutex,  // for conditional locks: do not lock 
-  DoLockMutex      // for conditional locks: do lock 
-};
-
 class MutexLocker {
 public:
-  MutexLocker(GooMutex *mutexA, MutexLockMode modeA = DoLockMutex) : mutex(mutexA), mode(modeA) { if (mode == DoLockMutex) gLockMutex(mutex); }
-  ~MutexLocker() { if (mode == DoLockMutex) gUnlockMutex(mutex); }
+  MutexLocker(GooMutex *mutexA) : mutex(mutexA) { gLockMutex(mutex); }
+  ~MutexLocker() { gUnlockMutex(mutex); }
 
 private:
   GooMutex *mutex;
-  const MutexLockMode mode;
 };
 
 #endif
diff --git a/poppler/Annot.cc b/poppler/Annot.cc
index f1181a7..2713fde 100644
--- a/poppler/Annot.cc
+++ b/poppler/Annot.cc
@@ -1213,7 +1213,7 @@ void Annot::initialize(PDFDoc *docA, Dict *dict) {
   if (dict->lookupNF("P", &obj1)->isRef()) {
     Ref ref = obj1.getRef();
 
-    page = doc->getCatalog()->findPage (ref.num, ref.gen, DoNotLockMutex);
+    page = doc->getCatalog()->findPage (ref.num, ref.gen);
   } else {
     page = 0;
   }
@@ -1350,8 +1350,8 @@ GBool Annot::inRect(double x, double y) const {
   return rect->contains(x, y);
 }
 
-void Annot::update(const char *key, Object *value, MutexLockMode lock) {
-  annotCondLocker(lock);
+void Annot::update(const char *key, Object *value) {
+  annotLocker();
   /* Set M to current time, unless we are updating M itself */
   if (strcmp(key, "M") != 0) {
     delete modified;
@@ -1384,7 +1384,7 @@ void Annot::setContents(GooString *new_content) {
   
   Object obj1;
   obj1.initString(contents->copy());
-  update ("Contents", &obj1, DoNotLockMutex);
+  update ("Contents", &obj1);
 }
 
 void Annot::setName(GooString *new_name) {
@@ -1399,7 +1399,7 @@ void Annot::setName(GooString *new_name) {
 
   Object obj1;
   obj1.initString(name->copy());
-  update ("NM", &obj1, DoNotLockMutex);
+  update ("NM", &obj1);
 }
 
 void Annot::setModified(GooString *new_modified) {
@@ -1413,7 +1413,7 @@ void Annot::setModified(GooString *new_modified) {
 
   Object obj1;
   obj1.initString(modified->copy());
-  update ("M", &obj1, DoNotLockMutex);
+  update ("M", &obj1);
 }
 
 void Annot::setFlags(Guint new_flags) {
@@ -1421,7 +1421,7 @@ void Annot::setFlags(Guint new_flags) {
   Object obj1;
   flags = new_flags;
   obj1.initInt(flags);
-  update ("F", &obj1, DoNotLockMutex);
+  update ("F", &obj1);
 }
 
 void Annot::setBorder(AnnotBorderArray *new_border) {
@@ -1431,7 +1431,7 @@ void Annot::setBorder(AnnotBorderArray *new_border) {
   if (new_border) {
     Object obj1;
     new_border->writeToObject(xref, &obj1);
-    update ("Border", &obj1, DoNotLockMutex);
+    update ("Border", &obj1);
     border = new_border;
   } else {
     border = NULL;
@@ -1445,7 +1445,7 @@ void Annot::setColor(AnnotColor *new_color) {
   if (new_color) {
     Object obj1;
     new_color->writeToObject(xref, &obj1);
-    update ("C", &obj1, DoNotLockMutex);
+    update ("C", &obj1);
     color = new_color;
   } else {
     color = NULL;
@@ -1467,12 +1467,12 @@ void Annot::setPage(int pageIndex, GBool updateP) {
   }
 
   if (updateP) {
-    update("P", &obj1, DoNotLockMutex);
+    update("P", &obj1);
   }
 }
 
-void Annot::setAppearanceState(const char *state, MutexLockMode lock) {
-  annotCondLocker(lock);
+void Annot::setAppearanceState(const char *state) {
+  annotLocker();
   if (!state)
     return;
 
@@ -1484,7 +1484,7 @@ void Annot::setAppearanceState(const char *state, MutexLockMode lock) {
 
   Object obj1;
   obj1.initName(state);
-  update ("AS", &obj1, DoNotLockMutex);
+  update ("AS", &obj1);
 
   // The appearance state determines the current appearance stream
   appearance.free();
@@ -1503,12 +1503,12 @@ void Annot::invalidateAppearance() {
   delete appearStreams;
   appearStreams = NULL;
 
-  setAppearanceState("Off", DoNotLockMutex); // Default appearance state
+  setAppearanceState("Off"); // Default appearance state
 
   Object obj1;
   obj1.initNull();
-  update ("AP", &obj1, DoNotLockMutex); // Remove AP
-  update ("AS", &obj1, DoNotLockMutex); // Remove AS
+  update ("AP", &obj1); // Remove AP
+  update ("AS", &obj1); // Remove AS
 }
 
 double Annot::getXMin() {
@@ -3813,7 +3813,7 @@ AnnotWidget::~AnnotWidget() {
 void AnnotWidget::initialize(PDFDoc *docA, Dict *dict) {
   Object obj1;
 
-  form = doc->getCatalog()->getForm(DoNotLockMutex);
+  form = doc->getCatalog()->getForm();
 
   if(dict->lookup("H", &obj1)->isName()) {
     const char *modeName = obj1.getName();
diff --git a/poppler/Annot.h b/poppler/Annot.h
index 7bc76b7..7be2114 100644
--- a/poppler/Annot.h
+++ b/poppler/Annot.h
@@ -572,7 +572,7 @@ public:
   // new_color. 
   void setColor(AnnotColor *new_color);
 
-  void setAppearanceState(const char *state, MutexLockMode lock = DoLockMutex);
+  void setAppearanceState(const char *state);
 
   // Delete appearance streams and reset appearance state
   void invalidateAppearance();
@@ -627,7 +627,7 @@ protected:
 
   // Updates the field key of the annotation dictionary
   // and sets M to the current time
-  void update(const char *key, Object *value, MutexLockMode lock = DoLockMutex);
+  void update(const char *key, Object *value);
 
   int refCnt;
 
diff --git a/poppler/Catalog.cc b/poppler/Catalog.cc
index 9c5fb71..f49049a 100644
--- a/poppler/Catalog.cc
+++ b/poppler/Catalog.cc
@@ -58,10 +58,8 @@
 
 #if MULTITHREADED
 #  define catalogLocker()   MutexLocker locker(&mutex)
-#  define catalogCondLocker(X)  MutexLocker locker(&mutex, (X))
 #else
 #  define catalogLocker()
-#  define catalogCondLocker(X)
 #endif
 //------------------------------------------------------------------------
 // Catalog
@@ -234,11 +232,11 @@ Page *Catalog::getPage(int i)
   return pages[i-1];
 }
 
-Ref *Catalog::getPageRef(int i, MutexLockMode lock)
+Ref *Catalog::getPageRef(int i)
 {
   if (i < 1) return NULL;
 
-  catalogCondLocker(lock);
+  catalogLocker();
   if (i > lastCachedPage) {
      GBool cached = cachePageTree(i);
      if ( cached == gFalse) {
@@ -294,7 +292,7 @@ GBool Catalog::cachePageTree(int page)
       return gFalse;
     }
 
-    pagesSize = getNumPages(DoNotLockMutex);
+    pagesSize = getNumPages();
     pages = (Page **)gmallocn(pagesSize, sizeof(Page *));
     pageRefs = (Ref *)gmallocn(pagesSize, sizeof(Ref));
     for (int i = 0; i < pagesSize; ++i) {
@@ -421,11 +419,11 @@ GBool Catalog::cachePageTree(int page)
   return gFalse;
 }
 
-int Catalog::findPage(int num, int gen, MutexLockMode lock) {
+int Catalog::findPage(int num, int gen) {
   int i;
 
-  for (i = 0; i < getNumPages(lock); ++i) {
-    Ref *ref = getPageRef(i+1, lock);
+  for (i = 0; i < getNumPages(); ++i) {
+    Ref *ref = getPageRef(i+1);
     if (ref != NULL && ref->num == num && ref->gen == gen)
       return i + 1;
   }
@@ -772,9 +770,9 @@ GBool Catalog::indexToLabel(int index, GooString *label)
   }
 }
 
-int Catalog::getNumPages(MutexLockMode lock)
+int Catalog::getNumPages()
 {
-  catalogCondLocker((numPages == -1 && lock == DoLockMutex) ? DoLockMutex : DoNotLockMutex);
+  catalogLocker();
   if (numPages == -1)
   {
     Object catDict, pagesDict, obj;
@@ -829,7 +827,7 @@ PageLabelInfo *Catalog::getPageLabelInfo()
     }
 
     if (catDict.dictLookup("PageLabels", &obj)->isDict()) {
-      pageLabelInfo = new PageLabelInfo(&obj, getNumPages(DoNotLockMutex));
+      pageLabelInfo = new PageLabelInfo(&obj, getNumPages());
     }
     obj.free();
     catDict.free();
@@ -916,9 +914,9 @@ Catalog::FormType Catalog::getFormType()
   return res;
 }
 
-Form *Catalog::getForm(MutexLockMode lock)
+Form *Catalog::getForm()
 {
-  catalogCondLocker(lock);
+  catalogLocker();
   if (!form) {
     if (acroForm.isDict()) {
       form = new Form(doc, &acroForm);
diff --git a/poppler/Catalog.h b/poppler/Catalog.h
index 4ee7bc9..24a3dcf 100644
--- a/poppler/Catalog.h
+++ b/poppler/Catalog.h
@@ -107,13 +107,13 @@ public:
   GBool isOk() { return ok; }
 
   // Get number of pages.
-  int getNumPages(MutexLockMode lock = DoLockMutex);
+  int getNumPages();
 
   // Get a page.
   Page *getPage(int i);
 
   // Get the reference for a page object.
-  Ref *getPageRef(int i, MutexLockMode lock = DoLockMutex);
+  Ref *getPageRef(int i);
 
   // Return base URI, or NULL if none.
   GooString *getBaseURI() { return baseURI; }
@@ -127,7 +127,7 @@ public:
 
   // Find a page, given its object ID.  Returns page number, or 0 if
   // not found.
-  int findPage(int num, int gen, MutexLockMode lock = DoLockMutex);
+  int findPage(int num, int gen);
 
   // Find a named destination.  Returns the link destination, or
   // NULL if <name> is not a destination.
@@ -165,7 +165,7 @@ public:
   };
 
   FormType getFormType();
-  Form* getForm(MutexLockMode lock = DoLockMutex);
+  Form* getForm();
 
   ViewerPreferences *getViewerPreferences();
 
diff --git a/poppler/XRef.cc b/poppler/XRef.cc
index 109917a..81ca9b1 100644
--- a/poppler/XRef.cc
+++ b/poppler/XRef.cc
@@ -1134,7 +1134,7 @@ Object *XRef::fetch(int num, int gen, Object *obj, int recursion) {
   Parser *parser;
   Object obj1, obj2, obj3;
 
-  xrefCondLocker((recursion == 0) ? DoLockMutex : DoNotLockMutex);
+  xrefLocker();
   // check for bogus ref - this can happen in corrupted PDF files
   if (num < 0 || num >= size) {
     goto err;
diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt
index 06378bd..609f7ed 100644
--- a/utils/CMakeLists.txt
+++ b/utils/CMakeLists.txt
@@ -36,6 +36,9 @@ if (HAVE_CAIRO)
   if(LCMS_FOUND)
     target_link_libraries(pdftocairo ${LCMS_LIBRARIES})
   endif(LCMS_FOUND)
+  if(HAVE_PTHREAD)
+    target_link_libraries(pdftocairo -lpthread)
+  endif()
   if(LCMS2_FOUND)
     target_link_libraries(pdftocairo ${LCMS2_LIBRARIES})
   endif(LCMS2_FOUND)


More information about the poppler mailing list