python: Make all CppOwnedPyObjects and similar support garbage collection.

If you want to subclass apt_pkg.Cache() and create an apt_pkg.DepCache()
object in it (e.g. as self.depcache) this is needed because otherwise,
Python would not know about the cyclic dependency and refuse to free any
of them.

This also changes apt_pkg.Cache to the standard deallocation schema,
because the underlying CacheFile deletes its pointers automatically on
deletion. Thus a second call is not needed.

8 files changed, 125 insertions, 70 deletions

diff --git a/python/acquire.cc b/python/acquire.cc
index 704ad0bd..b0dd2cab 100644
--- a/python/acquire.cc
+++ b/python/acquire.cc
@@ -372,10 +372,11 @@ PyTypeObject PkgAcquireFileType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    doc_PkgAcquireFile,                  // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgAcqFile*>,       // tp_traverse
+   CppOwnedClear<pkgAcqFile*>,          // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
diff --git a/python/cache.cc b/python/cache.cc
index a8da6696..4624dc34 100644
--- a/python/cache.cc
+++ b/python/cache.cc
@@ -241,19 +241,6 @@ static PyObject *CacheMapOp(PyObject *Self,PyObject *Arg)
    return CppOwnedPyObject_NEW<pkgCache::PkgIterator>(Self,&PackageType,Pkg);
 }
 
-// we need a special dealloc here to make sure that the CacheFile
-// is closed before deallocation the cache (otherwise we have a bad)
-// memory leak
-void PkgCacheFileDealloc(PyObject *Self)
-{
-   PyObject *CacheFilePy = GetOwner<pkgCache*>(Self);
-   pkgCacheFile *CacheF = GetCpp<pkgCacheFile*>(CacheFilePy);
-   CacheF->Close();
-   // Do not delete the pointer here, because it has already been deleted by
-   // closing the cache file.
-   CppOwnedDealloc<pkgCache *>(Self);
-}
-
 static PyObject *PkgCacheNew(PyTypeObject *type,PyObject *Args,PyObject *kwds)
 {
    PyObject *pyCallbackInst = 0;
@@ -320,7 +307,7 @@ PyTypeObject PkgCacheType =
    sizeof(CppOwnedPyObject<pkgCache *>),   // tp_basicsize
    0,                                   // tp_itemsize
    // Methods
-   PkgCacheFileDealloc,                  // tp_dealloc
+   CppOwnedDealloc<pkgCache *>,         // tp_dealloc
    0,                                   // tp_print
    0,                                   // tp_getattr
    0,                                   // tp_setattr
@@ -336,10 +323,11 @@ PyTypeObject PkgCacheType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    doc_PkgCache,                        // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgCache *>,        // tp_traverse
+   CppOwnedClear<pkgCache *>,           // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -460,7 +448,10 @@ PyTypeObject PkgListType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT ,                 // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
+   0,                                   // tp_doc
+   CppOwnedTraverse<PkgListStruct>,     // tp_traverse
+   CppOwnedClear<PkgListStruct>,        // tp_clear
 };
 
 #define MkGet(PyFunc,Ret) static PyObject *PyFunc(PyObject *Self,void*) \
@@ -581,10 +572,10 @@ PyTypeObject PackageType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                  // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
    "Package Object",                    // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgCache::PkgIterator>, // tp_traverse
+   CppOwnedClear<pkgCache::PkgIterator>,// tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -672,10 +663,10 @@ PyTypeObject DescriptionType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                  // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
    "apt_pkg.Description Object",        // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgCache::DescIterator>, // tp_traverse
+   CppOwnedClear<pkgCache::DescIterator>,// tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -935,10 +926,10 @@ PyTypeObject VersionType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                  // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,                  // tp_flags
    "Version Object",                    // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgCache::VerIterator>, // tp_traverse
+   CppOwnedClear<pkgCache::VerIterator>,// tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -1098,10 +1089,10 @@ PyTypeObject PackageFileType = {
    0,                                                    // tp_getattro
    0,                                                    // tp_setattro
    0,                                                    // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                                   // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,              // tp_flags
    "apt_pkg.PackageFile Object",                         // tp_doc
-   0,                                                    // tp_traverse
-   0,                                                    // tp_clear
+   CppOwnedTraverse<pkgCache::PkgFileIterator>,          // tp_traverse
+   CppOwnedClear<pkgCache::PkgFileIterator>,             // tp_clear
    0,                                                    // tp_richcompare
    0,                                                    // tp_weaklistoffset
    0,                                                    // tp_iter
@@ -1274,10 +1265,10 @@ PyTypeObject DependencyType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                  // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
    "Dependency Object",                 // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgCache::DepIterator>, // tp_traverse
+   CppOwnedClear<pkgCache::DepIterator>, // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -1362,7 +1353,10 @@ PyTypeObject RDepListType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                  // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
+   "DependencyList Object",             // tp_doc
+   CppOwnedTraverse<RDepListStruct>,    // tp_traverse
+   CppOwnedClear<RDepListStruct>,       // tp_clear
 };
 
 									/*}}}*/
diff --git a/python/depcache.cc b/python/depcache.cc
index 7cf8e0a2..d0b233b8 100644
--- a/python/depcache.cc
+++ b/python/depcache.cc
@@ -677,10 +677,11 @@ PyTypeObject PkgDepCacheType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    doc_PkgDepCache,                     // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgDepCache *>,     // tp_traverse
+   CppOwnedClear<pkgDepCache *>,        // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -866,10 +867,11 @@ PyTypeObject PkgProblemResolverType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    "ProblemResolver Object",            // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgProblemResolver *>, // tp_traverse
+   CppOwnedClear<pkgProblemResolver *>, // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -984,10 +986,11 @@ PyTypeObject PkgActionGroupType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    doc_PkgActionGroup,                  // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgDepCache::ActionGroup*>, // tp_traverse
+   CppOwnedClear<pkgDepCache::ActionGroup*>, // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -1002,7 +1005,7 @@ PyTypeObject PkgActionGroupType =
    0,                                   // tp_dictoffset
    0,                                   // tp_init
    0,                                   // tp_alloc
-   PkgActionGroupNew,                         // tp_new
+   PkgActionGroupNew,                   // tp_new
 };
 
 #ifdef COMPAT_0_7
diff --git a/python/generic.h b/python/generic.h
index b7b958f5..a9e6b8bf 100644
--- a/python/generic.h
+++ b/python/generic.h
@@ -29,6 +29,7 @@
 
 #include <Python.h>
 #include <string>
+#include <iostream>
 #include <new>
 
 #if PYTHON_API_VERSION < 1013
@@ -99,6 +100,9 @@ inline PyObject *GetOwner(PyObject *Obj)
 template <class T>
 inline CppPyObject<T> *CppPyObject_NEW(PyTypeObject *Type)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== ALLOCATING " << Type->tp_name << " ===\n";
+   #endif
    CppPyObject<T> *New = (CppPyObject<T>*)Type->tp_alloc(Type, 0);
    new (&New->Object) T;
    return New;
@@ -107,6 +111,9 @@ inline CppPyObject<T> *CppPyObject_NEW(PyTypeObject *Type)
 template <class T,class A>
 inline CppPyObject<T> *CppPyObject_NEW(PyTypeObject *Type,A const &Arg)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== ALLOCATING " << Type->tp_name << " ===\n";
+   #endif
    CppPyObject<T> *New = (CppPyObject<T>*)Type->tp_alloc(Type, 0);
    new (&New->Object) T(Arg);
    return New;
@@ -116,6 +123,9 @@ template <class T>
 inline CppOwnedPyObject<T> *CppOwnedPyObject_NEW(PyObject *Owner,
 						 PyTypeObject *Type)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== ALLOCATING " << Type->tp_name << "+ ===\n";
+   #endif
    CppOwnedPyObject<T> *New = (CppOwnedPyObject<T>*)Type->tp_alloc(Type, 0);
    new (&New->Object) T;
    New->Owner = Owner;
@@ -127,6 +137,9 @@ template <class T,class A>
 inline CppOwnedPyObject<T> *CppOwnedPyObject_NEW(PyObject *Owner,
 						 PyTypeObject *Type,A const &Arg)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== ALLOCATING " << Type->tp_name << "+ ===\n";
+   #endif
    CppOwnedPyObject<T> *New = (CppOwnedPyObject<T>*)Type->tp_alloc(Type, 0);
    new (&New->Object) T(Arg);
    New->Owner = Owner;
@@ -135,10 +148,26 @@ inline CppOwnedPyObject<T> *CppOwnedPyObject_NEW(PyObject *Owner,
    return New;
 }
 
+// Traversal and Clean for owned objects
+template <class T>
+int CppOwnedTraverse(PyObject *self, visitproc visit, void* arg) {
+    Py_VISIT(((CppOwnedPyObject<T> *)self)->Owner);
+    return 0;
+}
+
+template <class T>
+int CppOwnedClear(PyObject *self) {
+    Py_CLEAR(((CppOwnedPyObject<T> *)self)->Owner);
+    return 0;
+}
+
 // Generic Dealloc type functions
 template <class T>
 void CppDealloc(PyObject *Obj)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== DEALLOCATING " << Obj->ob_type->tp_name << " ===\n";
+   #endif
    GetCpp<T>(Obj).~T();
    Obj->ob_type->tp_free(Obj);
 }
@@ -146,10 +175,12 @@ void CppDealloc(PyObject *Obj)
 template <class T>
 void CppOwnedDealloc(PyObject *iObj)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== DEALLOCATING " << iObj->ob_type->tp_name << "+ ===\n";
+   #endif
    CppOwnedPyObject<T> *Obj = (CppOwnedPyObject<T> *)iObj;
    Obj->Object.~T();
-   if (Obj->Owner != 0)
-      Py_DECREF(Obj->Owner);
+   CppOwnedClear<T>(iObj);
    iObj->ob_type->tp_free(iObj);
 }
 
@@ -158,6 +189,9 @@ void CppOwnedDealloc(PyObject *iObj)
 template <class T>
 void CppDeallocPtr(PyObject *Obj)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== DEALLOCATING " << Obj->ob_type->tp_name << "* ===\n";
+   #endif
    delete GetCpp<T>(Obj);
    Obj->ob_type->tp_free(Obj);
 }
@@ -165,10 +199,12 @@ void CppDeallocPtr(PyObject *Obj)
 template <class T>
 void CppOwnedDeallocPtr(PyObject *iObj)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== DEALLOCATING " << iObj->ob_type->tp_name << "*+ ===\n";
+   #endif
    CppOwnedPyObject<T> *Obj = (CppOwnedPyObject<T> *)iObj;
    delete Obj->Object;
-   if (Obj->Owner != 0)
-      Py_DECREF(Obj->Owner);
+   CppOwnedClear<T>(iObj);
    iObj->ob_type->tp_free(iObj);
 }
 
diff --git a/python/indexfile.cc b/python/indexfile.cc
index ba709872..ef88c2f0 100644
--- a/python/indexfile.cc
+++ b/python/indexfile.cc
@@ -114,10 +114,10 @@ PyTypeObject PackageIndexFileType =
    0,                                   // tp_getattro
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
-   Py_TPFLAGS_DEFAULT,                  // tp_flags
+   Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, // tp_flags
    "pkgIndexFile Object",               // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgIndexFile*>,     // tp_traverse
+   CppOwnedClear<pkgIndexFile*>,        // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
diff --git a/python/pkgrecords.cc b/python/pkgrecords.cc
index 1e36259d..4a3c80db 100644
--- a/python/pkgrecords.cc
+++ b/python/pkgrecords.cc
@@ -187,10 +187,11 @@ PyTypeObject PkgRecordsType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    "Records Object",                    // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<PkgRecordsStruct>,  // tp_traverse
+   CppOwnedClear<PkgRecordsStruct>,     // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
diff --git a/python/policy.cc b/python/policy.cc
index 992a1192..39bbc1a3 100644
--- a/python/policy.cc
+++ b/python/policy.cc
@@ -167,15 +167,16 @@ PyTypeObject PyPolicy_Type = {
     0,                                   // tp_setattro
     0,                                   // tp_as_buffer
     (Py_TPFLAGS_DEFAULT |                // tp_flags
-     Py_TPFLAGS_BASETYPE),
+     Py_TPFLAGS_BASETYPE |
+     Py_TPFLAGS_HAVE_GC),
     Policy_doc,                          // tp_doc
-    0,                                   // tp_traverse
-    0,                                   // tp_clear
+    CppOwnedTraverse<pkgPolicy*>,        // tp_traverse
+    CppOwnedClear<pkgPolicy*>,           // tp_clear
     0,                                   // tp_richcompare
     0,                                   // tp_weaklistoffset
     0,                                   // tp_iter
     0,                                   // tp_iternext
-    Policy_Methods,                // tp_methods
+    Policy_Methods,                      // tp_methods
     0,                                   // tp_members
     0,                                   // tp_getset
     0,                                   // tp_base
diff --git a/python/tag.cc b/python/tag.cc
index 05423e3d..74857fe5 100644
--- a/python/tag.cc
+++ b/python/tag.cc
@@ -34,7 +34,7 @@
 using namespace std;
 									/*}}}*/
 /* We need to keep a private copy of the data.. */
-struct TagSecData : public CppPyObject<pkgTagSection>
+struct TagSecData : public CppOwnedPyObject<pkgTagSection>
 {
    char *Data;
 };
@@ -47,6 +47,18 @@ struct TagFileData : public PyObject
    FileFd Fd;
 };
 
+// Traversal and Clean for owned objects
+int TagFileTraverse(PyObject *self, visitproc visit, void* arg) {
+    Py_VISIT(((TagFileData *)self)->Section);
+    return 0;
+}
+
+int TagFileClear(PyObject *self) {
+    Py_CLEAR(((TagFileData *)self)->Section);
+    return 0;
+}
+
+
 									/*}}}*/
 // TagSecFree - Free a Tag Section					/*{{{*/
 // ---------------------------------------------------------------------
@@ -55,7 +67,7 @@ void TagSecFree(PyObject *Obj)
 {
    TagSecData *Self = (TagSecData *)Obj;
    delete [] Self->Data;
-   CppDealloc<pkgTagSection>(Obj);
+   CppOwnedDealloc<pkgTagSection>(Obj);
 }
 									/*}}}*/
 // TagFileFree - Free a Tag File					/*{{{*/
@@ -63,8 +75,11 @@ void TagSecFree(PyObject *Obj)
 /* */
 void TagFileFree(PyObject *Obj)
 {
+   #ifdef ALLOC_DEBUG
+   std::cerr << "=== DEALLOCATING " << Obj->ob_type->tp_name << "^ ===\n";
+   #endif
    TagFileData *Self = (TagFileData *)Obj;
-   Py_DECREF((PyObject *)Self->Section);
+   TagFileClear(Obj);
    Self->Object.~pkgTagFile();
    Self->Fd.~FileFd();
    Py_DECREF(Self->File);
@@ -298,6 +313,8 @@ static PyObject *TagFileNew(PyTypeObject *type,PyObject *Args,PyObject *kwds)
    // Create the section
    New->Section = (TagSecData*)(&TagSecType)->tp_alloc(&TagSecType, 0);
    new (&New->Section->Object) pkgTagSection();
+   New->Section->Owner = New;
+   Py_INCREF(New->Section->Owner);
    New->Section->Data = 0;
 
    return HandleErrors(New);
@@ -441,10 +458,11 @@ PyTypeObject TagSecType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    doc_TagSec,                          // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   CppOwnedTraverse<pkgTagSection>,     // tp_traverse
+   CppOwnedClear<pkgTagSection>,         // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter
@@ -529,10 +547,11 @@ PyTypeObject TagFileType =
    0,                                   // tp_setattro
    0,                                   // tp_as_buffer
    (Py_TPFLAGS_DEFAULT |                // tp_flags
-    Py_TPFLAGS_BASETYPE),
+    Py_TPFLAGS_BASETYPE |
+    Py_TPFLAGS_HAVE_GC),
    doc_TagFile,                         // tp_doc
-   0,                                   // tp_traverse
-   0,                                   // tp_clear
+   TagFileTraverse,                     // tp_traverse
+   TagFileClear,                        // tp_clear
    0,                                   // tp_richcompare
    0,                                   // tp_weaklistoffset
    0,                                   // tp_iter