From 3c15d022a9bafe60e1ef1146fe850b0e61dfd9f5 Mon Sep 17 00:00:00 2001
From: "Gustavo J. A. M. Carneiro" <gjc@gnome.org>
Date: Wed, 20 Aug 2008 15:26:33 +0100
Subject: [PATCH] Add unit tests, fix a C++ object refcount leak.

---
 pybindgen/cppclass_typehandlers.py |  7 ++++
 tests/foo.h                        | 17 ++++++++-
 tests/foomodulegen.py              |  2 +
 tests/footest.py                   | 59 ++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/pybindgen/cppclass_typehandlers.py b/pybindgen/cppclass_typehandlers.py
index a4f6bee..938d8fd 100644
--- a/pybindgen/cppclass_typehandlers.py
+++ b/pybindgen/cppclass_typehandlers.py
@@ -780,6 +780,13 @@ class CppClassPtrReturnValue(CppClassReturnValueBase):
                    self.cpp_class.helper_class.name, const_cast_value))
 
             wrapper.after_call.write_code("%s->obj = %s;" % (py_name, const_cast_value))
+        
+            # We are already referencing the existing python wrapper,
+            # so we do not need a reference to the C++ object as well.
+            if self.caller_owns_return and \
+                    isinstance(self.cpp_class.memory_policy, cppclass.ReferenceCountingPolicy):
+                self.cpp_class.memory_policy.write_decref(wrapper.after_call, value)
+
             wrapper.after_call.write_code("Py_INCREF(%s);" % py_name)
             wrapper.after_call.unindent()
             wrapper.after_call.write_code("} else {")
diff --git a/tests/foo.h b/tests/foo.h
index 2148e59..74362e5 100644
--- a/tests/foo.h
+++ b/tests/foo.h
@@ -225,6 +225,7 @@ private:
     Foo *m_foo_ptr;
     Foo *m_foo_shared_ptr;
     Zbr *m_zbr;
+    Zbr *m_internal_zbr;
 
 public:
 
@@ -235,18 +236,24 @@ public:
         delete m_foo_ptr;
         if (m_zbr)
             m_zbr->Unref ();
+        if (m_internal_zbr) {
+            m_internal_zbr->Unref ();
+            m_internal_zbr = NULL;
+        }
     }
 
     SomeObject (std::string const prefix)
         : m_prefix (prefix), m_foo_ptr (0),
-          m_foo_shared_ptr (0), m_zbr (0)
+          m_foo_shared_ptr (0), m_zbr (0),
+          m_internal_zbr (new Zbr)
         {
             SomeObject::instance_count++;
         }
 
     SomeObject (int prefix_len)
         : m_prefix (prefix_len, 'X'), m_foo_ptr (0),
-          m_foo_shared_ptr (0), m_zbr (0)
+          m_foo_shared_ptr (0), m_zbr (0),
+          m_internal_zbr (new Zbr)
         {
             SomeObject::instance_count++;
         }
@@ -351,6 +358,12 @@ public:
             return NULL;
     }
 
+    // -#- @return(caller_owns_return=true) -#-
+    Zbr* get_internal_zbr () {
+        m_internal_zbr->Ref ();
+        return m_internal_zbr;
+    }
+
     // return reference counted object, caller does not own return
     // -#- @return(caller_owns_return=false) -#-
     Zbr* peek_zbr () { return m_zbr; }
diff --git a/tests/foomodulegen.py b/tests/foomodulegen.py
index c131dfe..591b5c5 100755
--- a/tests/foomodulegen.py
+++ b/tests/foomodulegen.py
@@ -74,6 +74,7 @@ int %s::custom_method_added_by_a_hook(int x)
         helper_class.add_post_generation_code("// this comment was written by a helper class hook function")
     Zbr.add_helper_class_hook(helper_class_hook)
 
+    Zbr.add_constructor([])
     Zbr.add_constructor([Parameter.new('std::string', 'datum')])
     Zbr.add_method('get_datum', ReturnValue.new('std::string'), [])
     Zbr.add_method('get_int', ReturnValue.new('int'), [Parameter.new('int', 'x')],
@@ -219,6 +220,7 @@ int %s::custom_method_added_by_a_hook(int x)
 
     ## get/set recfcounted object Zbr
     SomeObject.add_method('get_zbr', ReturnValue.new('Zbr*', caller_owns_return=True), [])
+    SomeObject.add_method('get_internal_zbr', ReturnValue.new('Zbr*', caller_owns_return=True), [])
     SomeObject.add_method('peek_zbr', ReturnValue.new('Zbr*', caller_owns_return=False), [])
     SomeObject.add_method('set_zbr_transfer', ReturnValue.new('void'),
                           [Parameter.new('Zbr*', 'zbr', transfer_ownership=True)])
diff --git a/tests/footest.py b/tests/footest.py
index aba9940..5136f42 100644
--- a/tests/footest.py
+++ b/tests/footest.py
@@ -909,5 +909,64 @@ class TestFoo(unittest.TestCase):
         self.assertEqual(s1.xpto, 123)
         
 
+    def test_refcounting_v2(self):
+        zbr_count_before = foo.Zbr.instance_count
+
+        obj = foo.SomeObject("")
+        z1 = obj.get_internal_zbr()
+        z2 = obj.get_internal_zbr()
+        del obj, z1, z2
+
+        while gc.collect():
+            pass
+
+        zbr_count_after = foo.Zbr.instance_count
+
+        self.assertEqual(zbr_count_after, zbr_count_before)
+
+    def test_refcounting_v3(self):
+        zbr_count_before = foo.Zbr.instance_count
+        
+        class MyZbr(foo.Zbr):
+            pass
+        z_in = MyZbr()
+        obj = foo.SomeObject("")
+        obj.set_zbr_transfer(z_in)
+        del z_in
+        
+        z1 = obj.get_zbr()
+        z2 = obj.get_zbr()
+        self.assert_(z1 is z2)
+        del obj, z1, z2
+
+        while gc.collect():
+            pass
+
+        zbr_count_after = foo.Zbr.instance_count
+
+        self.assertEqual(zbr_count_after, zbr_count_before)
+
+    def test_refcounting_v4(self): # same as v3 but using peek_zbr instead of get_zbr
+        zbr_count_before = foo.Zbr.instance_count
+        
+        class MyZbr(foo.Zbr):
+            pass
+        z_in = MyZbr()
+        obj = foo.SomeObject("")
+        obj.set_zbr_transfer(z_in)
+        del z_in
+        
+        z1 = obj.peek_zbr()
+        z2 = obj.peek_zbr()
+        self.assert_(z1 is z2)
+        del obj, z1, z2
+
+        while gc.collect():
+            pass
+
+        zbr_count_after = foo.Zbr.instance_count
+
+        self.assertEqual(zbr_count_after, zbr_count_before)
+
 if __name__ == '__main__':
     unittest.main()