diff --git a/python/descriptor.c b/python/descriptor.c index 41c552038a..6a0d9cfcf6 100644 --- a/python/descriptor.c +++ b/python/descriptor.c @@ -72,7 +72,7 @@ static PyObject *PyUpb_DescriptorBase_NewInternal(PyTypeObject *type, static void PyUpb_DescriptorBase_Dealloc(PyUpb_DescriptorBase *self) { PyUpb_DescriptorBase *base = (PyUpb_DescriptorBase *)self; PyUpb_ObjCache_Delete(base->def); - Py_CLEAR(base->pool); + Py_DECREF(base->pool); PyObject_Del(self); } diff --git a/python/descriptor_pool.c b/python/descriptor_pool.c index fa9388b19c..adb70ef24e 100644 --- a/python/descriptor_pool.c +++ b/python/descriptor_pool.c @@ -46,12 +46,24 @@ static PyObject* PyUpb_DescriptorPool_DoCreate(PyTypeObject* type, PyUpb_DescriptorPool* pool = PyObject_New(PyUpb_DescriptorPool, type); pool->symtab = upb_symtab_new(); pool->db = db; + Py_XINCREF(pool->db); return &pool->ob_base; } +static int PyUpb_DescriptorPool_Traverse(PyUpb_DescriptorPool* self, + visitproc visit, void* arg) { + Py_VISIT(self->db); + return 0; +} + +static int PyUpb_DescriptorPool_Clear(PyUpb_DescriptorPool* self) { + Py_CLEAR(self->db); + return 0; +} + static void PyUpb_DescriptorPool_Dealloc(PyUpb_DescriptorPool* self) { upb_symtab_free(self->symtab); - Py_CLEAR(self->db); + PyUpb_DescriptorPool_Clear(self); PyObject_Del(self); } @@ -105,7 +117,7 @@ static PyObject* PyUpb_DescriptorPool_AddSerializedFile( google_protobuf_FileDescriptorProto_parse(buf, size, arena); if (!proto) { PyErr_SetString(PyExc_TypeError, "Couldn't parse file content!"); - return NULL; + goto done; } upb_status status; @@ -116,7 +128,7 @@ static PyObject* PyUpb_DescriptorPool_AddSerializedFile( PyErr_Format(PyExc_TypeError, "Couldn't build proto file into descriptor pool: %s", upb_status_errmsg(&status)); - return NULL; + goto done; } result = PyUpb_FileDescriptor_GetOrCreateWrapper(filedef, _self); @@ -137,9 +149,7 @@ static PyObject* PyUpb_DescriptorPool_FindExtensionByName(PyObject* _self, PyUpb_DescriptorPool* self = (PyUpb_DescriptorPool*)_self; const char* name = PyUpb_GetStrData(arg); - if (!name) { - return NULL; - } + if (!name) return NULL; const upb_fielddef* field = upb_symtab_lookupext(self->symtab, name); if (field == NULL) { @@ -186,16 +196,18 @@ static PyMethodDef PyUpb_DescriptorPool_Methods[] = { {NULL}}; static PyType_Slot PyUpb_DescriptorPool_Slots[] = { - {Py_tp_new, PyUpb_DescriptorPool_New}, + {Py_tp_clear, PyUpb_DescriptorPool_Clear}, {Py_tp_dealloc, PyUpb_DescriptorPool_Dealloc}, {Py_tp_methods, PyUpb_DescriptorPool_Methods}, + {Py_tp_new, PyUpb_DescriptorPool_New}, + {Py_tp_traverse, PyUpb_DescriptorPool_Traverse}, {0, NULL}}; static PyType_Spec PyUpb_DescriptorPool_Spec = { - PYUPB_MODULE_NAME ".DescriptorPool", // tp_name - sizeof(PyUpb_DescriptorPool), // tp_basicsize - 0, // tp_itemsize - Py_TPFLAGS_DEFAULT, // tp_flags + PYUPB_MODULE_NAME ".DescriptorPool", + sizeof(PyUpb_DescriptorPool), + 0, // tp_itemsize + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, PyUpb_DescriptorPool_Slots, }; diff --git a/python/minimal_test.py b/python/minimal_test.py index ce17ee70e2..072250a622 100644 --- a/python/minimal_test.py +++ b/python/minimal_test.py @@ -24,7 +24,7 @@ # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -"""A bare-bones unit test, to be removed once upb can pass existing unit tests.""" +"""A bare-bones unit test that doesn't load any generated code.""" import unittest diff --git a/python/protobuf.h b/python/protobuf.h index 76386f81bc..dd00e25424 100644 --- a/python/protobuf.h +++ b/python/protobuf.h @@ -79,9 +79,14 @@ PyUpb_ModuleState *PyUpb_ModuleState_Get(void); // remove itself from the map when it is destroyed. The map is weak so it does // not take references to the cached objects. +// Adds the given object to the cache, indexed by the given key. void PyUpb_ObjCache_Add(const void *key, PyObject *py_obj); + +// Removes the given key from the cache. It must exist in the cache currently. void PyUpb_ObjCache_Delete(const void *key); -PyObject *PyUpb_ObjCache_Get(const void *key); // returns NULL if not present. + +// Returns a new reference to an object if it exists, otherwise returns NULL. +PyObject *PyUpb_ObjCache_Get(const void *key); // ----------------------------------------------------------------------------- // Utilities