[pypy-commit] pypy fix-cpyext-releasebuffer: (antocuni, rlamy): modify an existing test to show broken behavior: the correct way to implement bf_getbuffer is to do a Py_INCREF(obj), which the broken test didn't. Once you do, you notice that cpyext never does the corresponding decref, thus causing a leak because the original object is never deallocated

[antocuni]

Author: Antonio Cuni <anto.cuni at gmail.com>
Branch: fix-cpyext-releasebuffer
Changeset: r90432:0d60d6a74fdc
Date: 2017-02-28 23:09 +0100
http://bitbucket.org/pypy/pypy/changeset/0d60d6a74fdc/

Log:	(antocuni, rlamy): modify an existing test to show broken behavior:
	the correct way to implement bf_getbuffer is to do a Py_INCREF(obj),
	which the broken test didn't. Once you do, you notice that cpyext
	never does the corresponding decref, thus causing a leak because the
	original object is never deallocated

diff --git a/pypy/module/cpyext/test/test_bufferobject.py b/pypy/module/cpyext/test/test_bufferobject.py
--- a/pypy/module/cpyext/test/test_bufferobject.py
+++ b/pypy/module/cpyext/test/test_bufferobject.py
@@ -73,16 +73,27 @@
                 return obj;
              """),
             ("get_cnt", "METH_NOARGS",
-             'return PyLong_FromLong(cnt);')], prologue="""
+             'return PyLong_FromLong(cnt);'),
+            ("get_dealloc_cnt", "METH_NOARGS",
+             'return PyLong_FromLong(dealloc_cnt);'),
+        ],
+        prologue="""
                 static float test_data = 42.f;
                 static int cnt=0;
+                static int dealloc_cnt=0;
                 static PyHeapTypeObject * type=NULL;
 
+                void dealloc(PyObject *self) {
+                    dealloc_cnt++;
+                }
                 int getbuffer(PyObject *obj, Py_buffer *view, int flags) {
 
                     cnt ++;
                     memset(view, 0, sizeof(Py_buffer));
                     view->obj = obj;
+                    /* see the CPython docs for why we need this incref:
+                       https://docs.python.org/3.5/c-api/typeobj.html#c.PyBufferProcs.bf_getbuffer */
+                    Py_INCREF(obj);
                     view->ndim = 0;
                     view->buf = (void *) &test_data;
                     view->itemsize = sizeof(float);
@@ -96,7 +107,7 @@
                 void releasebuffer(PyObject *obj, Py_buffer *view) { 
                     cnt --;
                 }
-            """, more_init="""
+            """, more_init="""            
                 type = (PyHeapTypeObject *) PyType_Type.tp_alloc(&PyType_Type, 0);
 
                 type->ht_type.tp_name = "Test";
@@ -106,6 +117,7 @@
                                           Py_TPFLAGS_HEAPTYPE | Py_TPFLAGS_HAVE_NEWBUFFER;
                 type->ht_type.tp_flags &= ~Py_TPFLAGS_HAVE_GC;
 
+                type->ht_type.tp_dealloc = dealloc;
                 type->ht_type.tp_as_buffer = &type->as_buffer;
                 type->as_buffer.bf_getbuffer = getbuffer;
                 type->as_buffer.bf_releasebuffer = releasebuffer;
@@ -116,6 +128,8 @@
         assert module.get_cnt() == 0
         a = memoryview(module.create_test())
         assert module.get_cnt() == 1
+        assert module.get_dealloc_cnt() == 0
         del a
-        gc.collect(); gc.collect(); gc.collect()
+        self.debug_collect()
         assert module.get_cnt() == 0
+        assert module.get_dealloc_cnt() == 1