diff --git a/ChangeLog b/ChangeLog index b17540a7..1caf6ed7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2010-06-06 Jay Berkenbilt + + * Fix memory leak for QPDF objects whose underlying PDF objects + contain circular references. Thanks to Jian Ma + for calling my attention to the memory leak. + 2010-04-25 Jay Berkenbilt * 2.1.5: release diff --git a/TODO b/TODO index 9776c73c..b18c459e 100644 --- a/TODO +++ b/TODO @@ -1,47 +1,3 @@ -Bug -=== - - * There is a memory leak that happens whenever object A refers to - object B which refers back to object A. We have the following - pattern: - - #include "PointerHolder.hh" - - class A - { - public: - PointerHolder a; - }; - - int main() - { - A a1; - a1.a = new A(); - a1.a.getPointer()->a = new A(); - a1.a.getPointer()->a.getPointer()->a = a1.a; - return 0; - } - - In order to fix this, we have to explicitly break circular - references, but we have to do this at a time that doesn't - completely destroy performance. - - To debug, configure with - - ./configure --disable-shared --disable-test-compare-images \ - CFLAGS=-g CXXFLAGS=-g - - Current code causes test failures. linearize segfaults on - hybrid-xref.pdf, and the test suite fails in various places. Maybe - because we don't do any kind of loop detection? - - Use valgrind --leak-check=full to see leak details. - - The file memory-leak.pdf is a minimal case that shows the problem. - - Files in trace having tracing to show which objects haven't been - allocated. - Next ==== diff --git a/include/qpdf/QPDFObject.hh b/include/qpdf/QPDFObject.hh index 26b15ef9..b366c33f 100644 --- a/include/qpdf/QPDFObject.hh +++ b/include/qpdf/QPDFObject.hh @@ -12,11 +12,33 @@ #include +class QPDF; +class QPDFObjectHandle; + class QPDFObject { public: virtual ~QPDFObject() {} virtual std::string unparse() = 0; + + // Accessor to give specific access to non-public methods + class ObjAccessor + { + friend class QPDF; + friend class QPDFObjectHandle; + private: + static void releaseResolved(QPDFObject* o) + { + if (o) + { + o->releaseResolved(); + } + } + }; + friend class ObjAccessor; + + protected: + virtual void releaseResolved() {} }; #endif // __QPDFOBJECT_HH__ diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index a62c85ab..7feb602d 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -22,6 +22,8 @@ class Pipeline; class QPDF; +class QPDF_Dictionary; +class QPDF_Array; class QPDFObjectHandle { @@ -247,6 +249,20 @@ class QPDFObjectHandle }; friend class ObjAccessor; + // Provide access to specific classes for recursive + // reverseResolved(). + class ReleaseResolver + { + friend class QPDF_Dictionary; + friend class QPDF_Array; + private: + static void releaseResolved(QPDFObjectHandle& o) + { + o.releaseResolved(); + } + }; + friend class ReleaseResolver; + private: QPDFObjectHandle(QPDF*, int objid, int generation); QPDFObjectHandle(QPDFObject*); @@ -262,6 +278,7 @@ class QPDFObjectHandle void assertPageObject(); void dereference(); void makeDirectInternal(std::set& visited); + void releaseResolved(); bool initialized; diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 44ea7d24..f68efc83 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -277,6 +277,13 @@ QPDF::QPDF() : QPDF::~QPDF() { + this->xref_table.clear(); + for (std::map::iterator iter = this->obj_cache.begin(); + iter != obj_cache.end(); ++iter) + { + QPDFObject::ObjAccessor::releaseResolved( + (*iter).second.object.getPointer()); + } } void diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 6fb66d9c..022c9e05 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -41,6 +41,22 @@ QPDFObjectHandle::QPDFObjectHandle(QPDFObject* data) : { } +void +QPDFObjectHandle::releaseResolved() +{ + if (isIndirect()) + { + if (this->obj.getPointer()) + { + this->obj = 0; + } + } + else + { + QPDFObject::ObjAccessor::releaseResolved(this->obj.getPointer()); + } +} + bool QPDFObjectHandle::isInitialized() const { diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc index b7227f0d..ae70bd67 100644 --- a/libqpdf/QPDF_Array.cc +++ b/libqpdf/QPDF_Array.cc @@ -10,6 +10,16 @@ QPDF_Array::~QPDF_Array() { } +void +QPDF_Array::releaseResolved() +{ + for (std::vector::iterator iter = this->items.begin(); + iter != this->items.end(); ++iter) + { + QPDFObjectHandle::ReleaseResolver::releaseResolved(*iter); + } +} + std::string QPDF_Array::unparse() { diff --git a/libqpdf/QPDF_Dictionary.cc b/libqpdf/QPDF_Dictionary.cc index 36c4590d..ccaab4a8 100644 --- a/libqpdf/QPDF_Dictionary.cc +++ b/libqpdf/QPDF_Dictionary.cc @@ -13,6 +13,17 @@ QPDF_Dictionary::~QPDF_Dictionary() { } +void +QPDF_Dictionary::releaseResolved() +{ + for (std::map::iterator iter = + this->items.begin(); + iter != this->items.end(); ++iter) + { + QPDFObjectHandle::ReleaseResolver::releaseResolved((*iter).second); + } +} + std::string QPDF_Dictionary::unparse() { diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh index fce36a4c..490df723 100644 --- a/libqpdf/qpdf/QPDF_Array.hh +++ b/libqpdf/qpdf/QPDF_Array.hh @@ -16,6 +16,9 @@ class QPDF_Array: public QPDFObject QPDFObjectHandle getItem(int n) const; void setItem(int, QPDFObjectHandle const&); + protected: + virtual void releaseResolved(); + private: std::vector items; }; diff --git a/libqpdf/qpdf/QPDF_Dictionary.hh b/libqpdf/qpdf/QPDF_Dictionary.hh index a6b1e77b..e75de01b 100644 --- a/libqpdf/qpdf/QPDF_Dictionary.hh +++ b/libqpdf/qpdf/QPDF_Dictionary.hh @@ -27,6 +27,9 @@ class QPDF_Dictionary: public QPDFObject // Remove key, doing nothing if key does not exist void removeKey(std::string const& key); + protected: + virtual void releaseResolved(); + private: std::map items; };