Commit 837e3704 authored by Albert Astals Cid's avatar Albert Astals Cid

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