fix memory leak

git-svn-id: svn+q:///qpdf/trunk@976 71b93d88-0707-0410-a8cf-f5a4172ac649

fix memory leak
git-svn-id: svn+q:///qpdf/trunk@976 71b93d88-0707-0410-a8cf-f5a4172ac649
Jay Berkenbilt
1 parent 64dc7388
Showing 10 changed files with 95 additions and 44 deletions
ChangeLog
TODO
include/qpdf/QPDFObject.hh
include/qpdf/QPDFObjectHandle.hh
libqpdf/QPDF.cc
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDF_Array.cc
libqpdf/QPDF_Dictionary.cc
libqpdf/qpdf/QPDF_Array.hh
libqpdf/qpdf/QPDF_Dictionary.hh
+2010-06-06  Jay Berkenbilt  <ejb@ql.org>
+
+	* Fix memory leak for QPDF objects whose underlying PDF objects
+	contain circular references.  Thanks to Jian Ma
+	<stronghorse@tom.com> for calling my attention to the memory leak.
+
 2010-04-25  Jay Berkenbilt  <ejb@ql.org>
  
 	* 2.1.5: release
-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> 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
 ====
  
@@ -12,11 +12,33 @@
  
 #include <string>
  
+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__
@@ -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<int>& visited);
+    void releaseResolved();
  
     bool initialized;
  
@@ -277,6 +277,13 @@ QPDF::QPDF() :
  
 QPDF::~QPDF()
 {
+    this->xref_table.clear();
+    for (std::map<ObjGen, ObjCache>::iterator iter = this->obj_cache.begin();
+	 iter != obj_cache.end(); ++iter)
+    {
+	QPDFObject::ObjAccessor::releaseResolved(
+	    (*iter).second.object.getPointer());
+    }
 }
  
 void
@@ -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
 {
@@ -10,6 +10,16 @@ QPDF_Array::~QPDF_Array()
 {
 }
  
+void
+QPDF_Array::releaseResolved()
+{
+    for (std::vector<QPDFObjectHandle>::iterator iter = this->items.begin();
+	 iter != this->items.end(); ++iter)
+    {
+	QPDFObjectHandle::ReleaseResolver::releaseResolved(*iter);
+    }
+}
+
 std::string
 QPDF_Array::unparse()
 {
@@ -13,6 +13,17 @@ QPDF_Dictionary::~QPDF_Dictionary()
 {
 }
  
+void
+QPDF_Dictionary::releaseResolved()
+{
+    for (std::map<std::string, QPDFObjectHandle>::iterator iter =
+	     this->items.begin();
+	 iter != this->items.end(); ++iter)
+    {
+	QPDFObjectHandle::ReleaseResolver::releaseResolved((*iter).second);
+    }
+}
+
 std::string
 QPDF_Dictionary::unparse()
 {
@@ -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<QPDFObjectHandle> items;
 };
@@ -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<std::string, QPDFObjectHandle> items;
 };