Addressed PR comments.
diff --git a/python/descriptor.c b/python/descriptor.c
index 41c5520..6a0d9cf 100644
--- a/python/descriptor.c
+++ b/python/descriptor.c
@@ -72,7 +72,7 @@
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 fa9388b..adb70ef 100644
--- a/python/descriptor_pool.c
+++ b/python/descriptor_pool.c
@@ -46,12 +46,24 @@
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 @@
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 @@
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 @@
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 @@
{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 ce17ee7..072250a 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 76386f8..dd00e25 100644
--- a/python/protobuf.h
+++ b/python/protobuf.h
@@ -79,9 +79,14 @@
// 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
