Garbage collection problems with a c++ wrapper for a module
Greg Chapman
glc at well.com
Fri Jul 16 12:14:38 EDT 2004
On 15 Jul 2004 11:15:10 -0700, james.sapara at gmail.com (James S) wrote:
>
>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 */
>}
More information about the Python-list
mailing list