Garbage collection problems with a c++ wrapper for a module

[James S]

Hi,

Basically I've been fighting with this code for a few days now and
can't seem to work around this problem. Included is the output, the
program I use to get this error and the source code for my wrapper.
This is acually part of the project, libxmlconf on sourceforge. The
newest working version isn't there yet, and cvs is lagged by 6 hours
or so. So if you think you want to have a try at this I can tgz the
source for you. My libxmlconf.cpp wrapper is based on the xxmodule.c
included with the source for the 2.3.3 python. That also happens to be
the version I'm developing and testing python with.

Any help would be fantastic,

James

Output:
python test.py
############ TEST ############
['XMLConf', '__doc__', '__file__', '__name__']
['__class__', '__delattr__', '__doc__', '__getattribute__',
'__hash__', '__init__', '__new__', '__reduce__', '__reduce_ex__',
'__repr__', '__setattr__', '__str__']
NEW
P: init
NEW
P: init
<libxmlconf.XMLConf object at 0x402f2050> <libxmlconf.XMLConf object
at 0x402f2060>

You can use the following gay ness to eat
cheese after wor
And then you can poke your eyes our with a
pen.
Don't forget to write!

############ DONE TEST ############
ASFDA
ASFDA
ASFDA
ASFDA
Exception exceptions.TypeError: 'function takes exactly 0 arguments (1
given)' in 'garbage collection' ignored
Fatal Python error: unexpected exception during garbage collection
make: *** [test] Aborted

# note this error happes after the program is done, so it would appear
to be a gc problem. Also, I can get this error to happen before hand
by importing gc after I have loaded and make an instance of my
wrapper.


test.py:
import libxmlconf


_d = "############"


print _d, "TEST",_d

print dir( libxmlconf )
print dir( libxmlconf.XMLConf )

s = libxmlconf.XMLConf( "../../runtest.xml")
ss = libxmlconf.XMLConf()
print s,ss
print s.GetValue("root.help.coin")
print _d, "DONE TEST",_d

libxmlconf.cpp:
/* Use this file as a template to start implementing a module that
also declares object types. All occurrences of 'Xxo' should be
changed
to something reasonable for your objects. After that, all other
occurrences of 'xx' should be changed to something reasonable for
your
module. If your module is named foo your sourcefile should be named
foomodule.c.

You will probably want to delete all references to 'x_attr' and add
your own types of attributes instead. Maybe you want to name your
local variables other than 'self'. If your object type is needed
in
other files, you'll have to create a file "foobarobject.h"; see
intobject.h for an example. */

/* Xxo objects */
#include "libxmlconf.h"
#include "Python.h"

extern "C" {

};

static PyObject *ErrorObject;

typedef struct {
PyObject_HEAD
struct XMLCONF_STRUCT *root; /* Attributes dictionary */
int inited;
} XMLCONFObject;

extern PyTypeObject XMLCONF_Type;

//#define XMLCONFObject_Check(v) ((v)->ob_type == &XMLCONF_Type))

static XMLCONFObject *
newXMLCONFObject(PyObject *arg)
{
printf("NEW\n");
XMLCONFObject *self;
self = PyObject_New(XMLCONFObject, &XMLCONF_Type);
if (self == NULL)
return NULL;

self->inited = 0;
self->root = NULL;
return self;
}

static int
XMLCONF_Initialize(
XMLCONFObject *self,
PyObject *args,
PyObject *kwargs )
{
printf("P: init\n");
char *filename;
self->inited = 1;
self->root = XMLConf_Create( );
if (PyArg_ParseTuple(args,"" ) )
return 0;
if (PyArg_ParseTuple(args,"s", &filename ) ) {
XMLConf_ParseFile( self->root, filename );
self->inited = 1;
return 0;
}

return -1;
}

static void
XMLCONF_dealloc(XMLCONFObject *self)
{
printf("DEALLOC");
if ( self->inited == 1 ) {
XMLConf_Destroy( self->root );
}
PyObject_Del(self);
printf(">DEALLOC\n");
}

static PyObject *
XMLCONF_GetValue(XMLCONFObject *self, PyObject *args)
{
PyObject *resultobj;
char *path;
char *result;
if ( PyArg_ParseTuple( args, "s", &path ) == 0 ) {
Py_INCREF(Py_None);
return Py_None;
}

result = XMLConf_GetValue( self->root, path );
if ( result == NULL ) {
Py_INCREF(Py_None);
return Py_None;
}
resultobj = Py_BuildValue("s", result );
Py_INCREF( resultobj );
return resultobj;
}
static PyObject *
XMLCONF_SetValue(XMLCONFObject *self, PyObject *args) {
Py_INCREF(Py_None);
}

static PyMethodDef XMLCONF_methods[] = {
{"GetValue", (PyCFunction)XMLCONF_GetValue, METH_VARARGS,
PyDoc_STR("Gets the Value")},
{"SetValue", (PyCFunction)XMLCONF_SetValue, METH_VARARGS,
PyDoc_STR("Sets the Value")},
{NULL, NULL} /* sentinel */
};

static PyObject *
XMLCONF_getattr(XMLCONFObject *self, char *name)
{
return Py_FindMethod(XMLCONF_methods, (PyObject *)self, name);
}

static int
XMLCONF_setattr(XMLCONFObject *self, char *name, PyObject *v)
{
printf("SETATTR\n");
return 0;
}

PyTypeObject XMLCONF_Type = {
/* The ob_type field must be initialized in the module init function
* to be portable to Windows without using C++. */
PyObject_HEAD_INIT(NULL)
0, /*ob_size*/
"libxmlconf.XMLConf", /*tp_name*/
sizeof(XMLCONFObject), /*tp_basicsize*/
0, /*tp_itemsize*/
/* methods */
(destructor)XMLCONF_dealloc, /*tp_dealloc*/
0, /*tp_print*/
(getattrfunc)XMLCONF_getattr, /*tp_getattr*/
(setattrfunc)XMLCONF_setattr, /*tp_setattr*/
0, /*tp_compare*/
0, /*tp_repr*/
0, /*tp_as_number*/
0, /*tp_as_sequence*/
0, /*tp_as_mapping*/
0, /*tp_hash*/
0, /*tp_call*/
0, /*tp_str*/
0, /*tp_getattro*/
0, /*tp_setattro*/
0, /*tp_as_buffer*/
Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE|Py_TPFLAGS_HAVE_GC,
/*tp_flags*/
0, /*tp_doc*/
0, /*tp_traverse*/
0, /*tp_clear*/
0, /*tp_richcompare*/
0, /*tp_weaklistoffset*/
0, /*tp_iter*/
0, /*tp_iternext*/
0, /*tp_methods*/
0, /*tp_members*/
0, /*tp_getset*/
0, /*tp_base*/
0, /*tp_dict*/
0, /*tp_descr_get*/
0, /*tp_descr_set*/
0, /*tp_dictoffset*/
(initproc) XMLCONF_Initialize,/*init*/
0, /*tp_alloc*/
(newfunc) newXMLCONFObject, /*tp_new*/
0, /*tp_free*/
0, /*tp_is_gc*/
};
/* ---------------------------------------------------------------------
*/

/* List of functions defined in the module */

static PyMethodDef xx_methods[] = {
{NULL, NULL} /* sentinel */
};

PyDoc_STRVAR(module_doc,
"This is a template module just for instruction.");

/* Initialization function for the module (*must* be called initxx) */

PyMODINIT_FUNC
initlibxmlconf(void)
{
PyObject *m;

/* Finalize the type object including setting type of the new type
* object; doing it here is required for portability to Windows
* without requiring C++. */
if (PyType_Ready(&XMLCONF_Type) < 0)
return;

/* Create the module and add the functions */
m = Py_InitModule3("libxmlconf", xx_methods, module_doc);
PyModule_AddObject( m, "XMLConf", (PyObject *)&XMLCONF_Type);
/* Add some symbolic constants to the module */
}

 

[Greg Chapman]

libxmlconf.cpp:

/* Xxo objects */
#include "libxmlconf.h"
#include "Python.h"

extern "C" {

};

static PyObject *ErrorObject;

typedef struct {
PyObject_HEAD
struct XMLCONF_STRUCT *root; /* Attributes dictionary */
int inited;
} XMLCONFObject;

extern PyTypeObject XMLCONF_Type;

//#define XMLCONFObject_Check(v) ((v)->ob_type == &XMLCONF_Type))

static XMLCONFObject *
newXMLCONFObject(PyObject *arg)
{
printf("NEW\n");
XMLCONFObject *self;
self = PyObject_New(XMLCONFObject, &XMLCONF_Type);
if (self == NULL)
return NULL;

self->inited = 0;
self->root = NULL;
return self;
}

In your type definition below, the above is used as your tp_new, but it doesn't
have the right signature. Also, the class is marked as supporting garbage
collection, but you're not using the GC allocator. But, as it turns out, you
don't need to support garbage collection in this class since it doesn't hold any
references to other python objects (and so can't create a reference cycle). So
it's better to simplify by removing the HAVE_GC flag. But, since you want this
to be a base class, you need to use type.tp_alloc to actually get the memory for
the instance (this allows a subclass to turn on garbage collection and have the
memory allocated by the GC allocator). Also, I would ditch the inited flag, and
simply call XMLConf_Create in the new method. A class invariant is then that
self->root is not NULL (once tp_new has completed).

(Caveat: I haven't tried to compile or test any of the below):

typedef struct {
PyObject_HEAD
struct XMLCONF_STRUCT *root; /* Attributes dictionary */
} XMLCONFObject;

PyObject*
newXMLCONFObject(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
XMLCONFObject *self;

printf("NEW\n");
self = (XMLCONFObject *)type->tp_alloc(type, 0);
if (self == NULL)
return NULL;
self->root = XMLConf_Create();
if (initialization of self->root failed) {
PyErr_SetString(some appropriate exception);
type->tp_free(self);
return NULL;
}
return (PyObject *)self;
}

static int
XMLCONF_Initialize(
XMLCONFObject *self,
PyObject *args,
PyObject *kwargs )
{
printf("P: init\n");
char *filename;
self->inited = 1;
self->root = XMLConf_Create( );
if (PyArg_ParseTuple(args,"" ) )
return 0;
if (PyArg_ParseTuple(args,"s", &filename ) ) {
XMLConf_ParseFile( self->root, filename );
self->inited = 1;
return 0;
}

return -1;
}

In the above, it looks like you want filename to be an optional argument; you
can specify it as such using a format of "|s". In the above, if the first call
to ParseTuple fails, it will set an exception. If the second call then
succeeds, the function returns successfully, but the exception is still set,
which can cause confusion down the line. In particular, I think this is the
exception which the garbage collector is detecting.

static int
XMLCONF_Initialize(
XMLCONFObject *self,
PyObject *args,
PyObject *kwargs )
{
static char *kwlist = {"filename", NULL};
char *filename = NULL;

printf("P: init\n");
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|s:XMLConf", kwlist,
&filename ))
return -1;

if (filename == NULL)
return 0;

XMLConf_ParseFile(self->root, filename);
if (ParseFile failed somehow) {
PyErr_RaiseAppropriateException;
return -1;
}
return 0;
}

Since you want this to be a base class, it is important that your dealloc
function calls the tp_free method defined in the actual type (self->ob_type) of
the instance; if the actual type is a subtype which supports garbage collection
this will call the GC deallocator.

static void
XMLCONF_dealloc(XMLCONFObject *self)
{
printf("DEALLOC");
XMLConf_Destroy( self->root );
self->ob_type->tp_free(self);
printf(">DEALLOC\n");
}

static PyObject *
XMLCONF_GetValue(XMLCONFObject *self, PyObject *args)
{
PyObject *resultobj;
char *path;
char *result;
if ( PyArg_ParseTuple( args, "s", &path ) == 0 ) {
Py_INCREF(Py_None);
return Py_None;
}

result = XMLConf_GetValue( self->root, path );
if ( result == NULL ) {
Py_INCREF(Py_None);
return Py_None;
}
resultobj = Py_BuildValue("s", result );
Py_INCREF( resultobj );
return resultobj;
}

First, if ParseTuple fails, an exception is set. In the above, if you really
want to return None in that case, you should clear the exception
(PyErr_Clear()). However, it seems much better to report the exception when no
argument is supplied. Also, Py_BuildValue returns a new reference suitable for
use as a method result. When you INCREF it, you add a reference which will
never be DECREF'd. And, anyway, it's easier to use PyString_FromString:

static PyObject *
XMLCONF_GetValue(XMLCONFObject *self, PyObject *args)
{
char *path;
char *result;
if ( !PyArg_ParseTuple( args, "s:GetValue", &path ))
return NULL;

result = XMLConf_GetValue( self->root, path );
if ( result == NULL ) {
/* perhaps an exception would be better here? */
Py_INCREF(Py_None);
return Py_None;
}
return PyString_FromString(result);
}

static PyObject *
XMLCONF_SetValue(XMLCONFObject *self, PyObject *args) {
Py_INCREF(Py_None);
}

I guess you want the above as a stub; you must remember to actually return
Py_None after INCREFing it. It's probably also worthwhile to at least check the
parameters, to make sure you got the right number:

static PyObject *
XMLCONF_SetValue(XMLCONFObject *self, PyObject *args) {
char *path, *value;

if (!PyArg_ParseTuple(args, "ss:SetValue", &path, &value))
return NULL;

Py_INCREF(Py_None);
return Py_None;
}

static PyMethodDef XMLCONF_methods[] = {
{"GetValue", (PyCFunction)XMLCONF_GetValue, METH_VARARGS,
PyDoc_STR("Gets the Value")},
{"SetValue", (PyCFunction)XMLCONF_SetValue, METH_VARARGS,
PyDoc_STR("Sets the Value")},
{NULL, NULL} /* sentinel */
};

It would be nice to supply more helpful docstrings; at the very least they
should indicate the method's signature.

static PyObject *
XMLCONF_getattr(XMLCONFObject *self, char *name)
{
return Py_FindMethod(XMLCONF_methods, (PyObject *)self, name);
}

static int
XMLCONF_setattr(XMLCONFObject *self, char *name, PyObject *v)
{
printf("SETATTR\n");
return 0;
}

You don't need the above anymore, provided you initialize the type's tp_methods
slot with XMLCONF_methods (and provided you call PyType_Ready as you are doing).
So the type object becomes:

PyTypeObject XMLCONF_Type = {
/* The ob_type field must be initialized in the module init function
* to be portable to Windows without using C++. */
PyObject_HEAD_INIT(NULL)
0, /*ob_size*/
"libxmlconf.XMLConf", /*tp_name*/
sizeof(XMLCONFObject), /*tp_basicsize*/
0, /*tp_itemsize*/
/* methods */
(destructor)XMLCONF_dealloc, /*tp_dealloc*/
0, /*tp_print*/
0, /*tp_getattr*/
0, /*tp_setattr*/
0, /*tp_compare*/
0, /*tp_repr*/
0, /*tp_as_number*/
0, /*tp_as_sequence*/
0, /*tp_as_mapping*/
0, /*tp_hash*/
0, /*tp_call*/
0, /*tp_str*/
0, /*tp_getattro*/
0, /*tp_setattro*/
0, /*tp_as_buffer*/
Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, /*tp_flags*/
0, /*tp_doc*/
0, /*tp_traverse*/
0, /*tp_clear*/
0, /*tp_richcompare*/
0, /*tp_weaklistoffset*/
0, /*tp_iter*/
0, /*tp_iternext*/
XMLCONF_methods, /*tp_methods*/
0, /*tp_members*/
0, /*tp_getset*/
0, /*tp_base*/
0, /*tp_dict*/
0, /*tp_descr_get*/
0, /*tp_descr_set*/
0, /*tp_dictoffset*/
(initproc) XMLCONF_Initialize,/*init*/
0, /*tp_alloc*/
newXMLCONFObject, /*tp_new*/
0, /*tp_free*/
0, /*tp_is_gc*/
};

/* List of functions defined in the module */

static PyMethodDef xx_methods[] = {
{NULL, NULL} /* sentinel */
};

PyDoc_STRVAR(module_doc,
"This is a template module just for instruction.");

/* Initialization function for the module (*must* be called initxx) */

PyMODINIT_FUNC
initlibxmlconf(void)
{
PyObject *m;

/* Finalize the type object including setting type of the new type
* object; doing it here is required for portability to Windows
* without requiring C++. */
if (PyType_Ready(&XMLCONF_Type) < 0)
return;

/* Create the module and add the functions */
m = Py_InitModule3("libxmlconf", xx_methods, module_doc);
PyModule_AddObject( m, "XMLConf", (PyObject *)&XMLCONF_Type);
/* Add some symbolic constants to the module */
}

 

[James S]

Wow. Thank you! Not only did you show be what was wrong, but you told
me what I was doing wrong. It all makes sense now. Thanks again. It
works perfect.

James