From bf027c48c8547f262f5ef5981c3ae44273ae249d Mon Sep 17 00:00:00 2001 From: Thomas Heller Date: Thu, 24 Apr 2008 18:14:19 +0000 Subject: [PATCH] Remove cyclic reference in CFuncPtr instances; see issue #2682. Backport candidate for the release25-maint branch. --- Misc/NEWS | 3 + Modules/_ctypes/_ctypes.c | 38 ++++------- Modules/_ctypes/callbacks.c | 129 ++++++++++++++++++++++++++++++------ Modules/_ctypes/ctypes.h | 18 +++-- 4 files changed, 137 insertions(+), 51 deletions(-) diff --git a/Misc/NEWS b/Misc/NEWS index 52aee8026fa..7862ffec514 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -43,6 +43,9 @@ Extensions Modules Library ------- +- Issue #2682: ctypes callback functions now longer contain a cyclic + reference to themselves. + - The getpass module has been improved on Unix. It now uses /dev/tty by default and uses stderr instead of stdout. A GetPassWarning is issued when input echo cannot be controlled. diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 355fda207f8..c9da7d3c1e7 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3096,7 +3096,7 @@ CFuncPtr_new(PyTypeObject *type, PyObject *args, PyObject *kwds) CFuncPtrObject *self; PyObject *callable; StgDictObject *dict; - ffi_info *thunk; + CThunkObject *thunk; if (PyTuple_GET_SIZE(args) == 0) return GenericCData_new(type, args, kwds); @@ -3153,11 +3153,6 @@ CFuncPtr_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; } - /*****************************************************************/ - /* The thunk keeps unowned references to callable and dict->argtypes - so we have to keep them alive somewhere else: callable is kept in self, - dict->argtypes is in the type's stgdict. - */ thunk = AllocFunctionCallback(callable, dict->argtypes, dict->restype, @@ -3166,27 +3161,22 @@ CFuncPtr_new(PyTypeObject *type, PyObject *args, PyObject *kwds) return NULL; self = (CFuncPtrObject *)GenericCData_new(type, args, kwds); - if (self == NULL) + if (self == NULL) { + Py_DECREF(thunk); return NULL; + } Py_INCREF(callable); self->callable = callable; self->thunk = thunk; - *(void **)self->b_ptr = *(void **)thunk; - - /* We store ourself in self->b_objects[0], because the whole instance - must be kept alive if stored in a structure field, for example. - Cycle GC to the rescue! And we have a unittest proving that this works - correctly... - */ - - Py_INCREF((PyObject *)self); /* for KeepRef */ - if (-1 == KeepRef((CDataObject *)self, 0, (PyObject *)self)) { + *(void **)self->b_ptr = (void *)thunk->pcl; + + Py_INCREF((PyObject *)thunk); /* for KeepRef */ + if (-1 == KeepRef((CDataObject *)self, 0, (PyObject *)thunk)) { Py_DECREF((PyObject *)self); return NULL; } - return (PyObject *)self; } @@ -3639,6 +3629,7 @@ CFuncPtr_traverse(CFuncPtrObject *self, visitproc visit, void *arg) Py_VISIT(self->argtypes); Py_VISIT(self->converters); Py_VISIT(self->paramflags); + Py_VISIT(self->thunk); return CData_traverse((CDataObject *)self, visit, arg); } @@ -3652,13 +3643,7 @@ CFuncPtr_clear(CFuncPtrObject *self) Py_CLEAR(self->argtypes); Py_CLEAR(self->converters); Py_CLEAR(self->paramflags); - - if (self->thunk) { - FreeClosure(self->thunk->pcl); - PyMem_Free(self->thunk); - self->thunk = NULL; - } - + Py_CLEAR(self->thunk); return CData_clear((CDataObject *)self); } @@ -5186,6 +5171,9 @@ init_ctypes(void) if (PyType_Ready(&PyCArg_Type) < 0) return; + if (PyType_Ready(&CThunk_Type) < 0) + return; + /* StgDict is derived from PyDict_Type */ StgDict_Type.tp_base = &PyDict_Type; if (PyType_Ready(&StgDict_Type) < 0) diff --git a/Modules/_ctypes/callbacks.c b/Modules/_ctypes/callbacks.c index 21637addbd0..0d3655e0937 100644 --- a/Modules/_ctypes/callbacks.c +++ b/Modules/_ctypes/callbacks.c @@ -12,6 +12,73 @@ #endif #include "ctypes.h" +/**************************************************************/ + +static CThunkObject_dealloc(PyObject *_self) +{ + CThunkObject *self = (CThunkObject *)_self; + Py_XDECREF(self->converters); + Py_XDECREF(self->callable); + Py_XDECREF(self->restype); + if (self->pcl) + FreeClosure(self->pcl); + PyObject_Del(self); +} + +static int +CThunkObject_traverse(PyObject *_self, visitproc visit, void *arg) +{ + CThunkObject *self = (CThunkObject *)_self; + Py_VISIT(self->converters); + Py_VISIT(self->callable); + Py_VISIT(self->restype); + return 0; +} + +static int +CThunkObject_clear(PyObject *_self) +{ + CThunkObject *self = (CThunkObject *)_self; + Py_CLEAR(self->converters); + Py_CLEAR(self->callable); + Py_CLEAR(self->restype); + return 0; +} + +PyTypeObject CThunk_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + "_ctypes.CThunkObject", + sizeof(CThunkObject), /* tp_basicsize */ + sizeof(ffi_type), /* tp_itemsize */ + CThunkObject_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, /* tp_flags */ + "CThunkObject", /* tp_doc */ + CThunkObject_traverse, /* tp_traverse */ + CThunkObject_clear, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + 0, /* tp_members */ +}; + +/**************************************************************/ + static void PrintError(char *msg, ...) { @@ -247,32 +314,56 @@ static void closure_fcn(ffi_cif *cif, void **args, void *userdata) { - ffi_info *p = userdata; + CThunkObject *p = (CThunkObject *)userdata; _CallPythonObject(resp, - p->restype, + p->ffi_restype, p->setfunc, p->callable, p->converters, args); } -ffi_info *AllocFunctionCallback(PyObject *callable, - PyObject *converters, - PyObject *restype, - int is_cdecl) +static CThunkObject* CThunkObject_new(Py_ssize_t nArgs) { - int result; - ffi_info *p; - Py_ssize_t nArgs, i; - ffi_abi cc; + CThunkObject *p; + int i; - nArgs = PySequence_Size(converters); - p = (ffi_info *)PyMem_Malloc(sizeof(ffi_info) + sizeof(ffi_type) * (nArgs)); + p = PyObject_NewVar(CThunkObject, &CThunk_Type, nArgs); if (p == NULL) { PyErr_NoMemory(); return NULL; } + + p->pcl = NULL; + memset(&p->cif, 0, sizeof(p->cif)); + p->converters = NULL; + p->callable = NULL; + p->setfunc = NULL; + p->ffi_restype = NULL; + + for (i = 0; i < nArgs + 1; ++i) + p->atypes[i] = NULL; + return p; +} + +CThunkObject *AllocFunctionCallback(PyObject *callable, + PyObject *converters, + PyObject *restype, + int is_cdecl) +{ + int result; + CThunkObject *p; + Py_ssize_t nArgs, i; + ffi_abi cc; + + nArgs = PySequence_Size(converters); + p = CThunkObject_new(nArgs); + if (p == NULL) + return NULL; + + assert(CThunk_CheckExact(p)); + p->pcl = MallocClosure(); if (p->pcl == NULL) { PyErr_NoMemory(); @@ -288,9 +379,11 @@ ffi_info *AllocFunctionCallback(PyObject *callable, } p->atypes[i] = NULL; + Py_INCREF(restype); + p->restype = restype; if (restype == Py_None) { p->setfunc = NULL; - p->restype = &ffi_type_void; + p->ffi_restype = &ffi_type_void; } else { StgDictObject *dict = PyType_stgdict(restype); if (dict == NULL || dict->setfunc == NULL) { @@ -299,7 +392,7 @@ ffi_info *AllocFunctionCallback(PyObject *callable, goto error; } p->setfunc = dict->setfunc; - p->restype = &dict->ffi_type_pointer; + p->ffi_restype = &dict->ffi_type_pointer; } cc = FFI_DEFAULT_ABI; @@ -323,16 +416,14 @@ ffi_info *AllocFunctionCallback(PyObject *callable, goto error; } + Py_INCREF(converters); p->converters = converters; + Py_INCREF(callable); p->callable = callable; return p; error: - if (p) { - if (p->pcl) - FreeClosure(p->pcl); - PyMem_Free(p); - } + Py_XDECREF(p); return NULL; } diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 8f6846a4476..1a104cfb2c2 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -84,14 +84,18 @@ struct tagCDataObject { }; typedef struct { + PyObject_VAR_HEAD ffi_closure *pcl; /* the C callable */ ffi_cif cif; PyObject *converters; PyObject *callable; + PyObject *restype; SETFUNC setfunc; - ffi_type *restype; + ffi_type *ffi_restype; ffi_type *atypes[1]; -} ffi_info; +} CThunkObject; +extern PyTypeObject CThunk_Type; +#define CThunk_CheckExact(v) ((v)->ob_type == &CThunk_Type) typedef struct { /* First part identical to tagCDataObject */ @@ -107,7 +111,7 @@ typedef struct { union value b_value; /* end of tagCDataObject, additional fields follow */ - ffi_info *thunk; + CThunkObject *thunk; PyObject *callable; /* These two fields will override the ones in the type's stgdict if @@ -178,10 +182,10 @@ extern void init_callbacks_in_module(PyObject *m); extern PyMethodDef module_methods[]; -extern ffi_info *AllocFunctionCallback(PyObject *callable, - PyObject *converters, - PyObject *restype, - int stdcall); +extern CThunkObject *AllocFunctionCallback(PyObject *callable, + PyObject *converters, + PyObject *restype, + int stdcall); /* a table entry describing a predefined ctypes type */ struct fielddesc { char code;