From dd161ca17edad129d887212bb28e49ccec3d4901 Mon Sep 17 00:00:00 2001 From: hantianfeng Date: Wed, 12 Jun 2024 20:59:58 +0800 Subject: [PATCH] Optimize reference count code, fix memory leak --- include/phpy.h | 1 + src/bridge/core.cc | 46 ++++++++++++++++++++++++---- src/php/dict.cc | 6 ++-- src/php/list.cc | 8 +++-- src/php/object.cc | 19 +++++++----- src/php/tuple.cc | 2 ++ tools/mem_leak.php | 74 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 137 insertions(+), 19 deletions(-) create mode 100644 tools/mem_leak.php diff --git a/include/phpy.h b/include/phpy.h index 9386c71..0811192 100644 --- a/include/phpy.h +++ b/include/phpy.h @@ -151,6 +151,7 @@ void debug_dump(uint32_t i, zval *item); void debug_dump(uint32_t i, PyObject *pObj); void var_dump(zval *var); void debug_var_dump(zval *var); +void debug_print_refcnt(const char *fn, PyObject *zv); bool py_module_string_init(PyObject *m); bool py_module_object_init(PyObject *m); diff --git a/src/bridge/core.cc b/src/bridge/core.cc index a92b34c..5acd700 100644 --- a/src/bridge/core.cc +++ b/src/bridge/core.cc @@ -118,6 +118,9 @@ void py2php_scalar(PyObject *pv, zval *zv) { py2php_fn(pv, zv); } +/** + * Increase reference count of the value + */ void py2php(PyObject *pv, zval *zv) { py2php_fn = py2php_object_impl; py2php_fn(pv, zv); @@ -206,7 +209,9 @@ PyObject *array2list(zend_array *ht) { zval *current; PyObject *list = PyList_New(0); ZEND_HASH_FOREACH_VAL(ht, current) { - PyList_Append(list, php2py(current)); + auto elem = php2py(current); + PyList_Append(list, elem); + Py_DECREF(elem); } ZEND_HASH_FOREACH_END(); return list; @@ -217,6 +222,8 @@ PyObject *array2tuple(zend_array *ht) { PyObject *tuple = PyTuple_New(phpy::php::array_count(ht)); Py_ssize_t index = 0; ZEND_HASH_FOREACH_VAL(ht, current) { + // PyTuple_SetItem() + // NOT increase reference count of the value PyTuple_SetItem(tuple, index++, php2py(current)); } ZEND_HASH_FOREACH_END(); @@ -227,7 +234,9 @@ PyObject *array2set(zend_array *ht) { zval *current; PyObject *pset = PySet_New(0); ZEND_HASH_FOREACH_VAL(ht, current) { - PySet_Add(pset, php2py(current)); + auto elem = php2py(current); + PySet_Add(pset, elem); + Py_DECREF(elem); } ZEND_HASH_FOREACH_END(); return pset; @@ -244,6 +253,7 @@ static void iterator2array(PyObject *pv, zval *zv) { zval item; py2php_fn(next, &item); add_next_index_zval(zv, &item); + Py_DECREF(next); } Py_DECREF(iter); } @@ -260,7 +270,14 @@ PyObject *array2dict(zend_array *ht) { } else { dk = PyLong_FromLong(index); } - PyDict_SetItem(dict, dk, php2py(value)); + auto elem = php2py(value); + /** + * PyDict_SetItem() + * Increase reference count of the key and value + */ + PyDict_SetItem(dict, dk, elem); + Py_DECREF(elem); + Py_DECREF(dk); } ZEND_HASH_FOREACH_END(); return dict; @@ -270,15 +287,24 @@ static void dict2array(PyObject *pv, zval *zv) { PyObject *iter = PyObject_GetIter(pv); array_init(zv); while (true) { + /** + * PyIter_Next() + * Return value: New reference + */ PyObject *next = PyIter_Next(iter); if (!next) { break; } + /** + * PyDict_GetItem() + * Return value: Borrowed reference + */ auto value = PyDict_GetItem(pv, next); zval item; py2php_fn(value, &item); StrObject key(next); add_assoc_zval_ex(zv, key.val(), key.len(), &item); + Py_DECREF(next); } Py_DECREF(iter); } @@ -374,6 +400,10 @@ void debug_var_dump(zval *var) { php_debug_zval_dump(var, var_dump_level); } +void debug_print_refcnt(const char *fn, PyObject *zv) { + printf("[%s] refcount=%zu\n", fn, Py_REFCNT(zv)); +} + CallObject::CallObject(PyObject *_fn, zval *_return_value, uint32_t _argc, zval *_argv, zend_array *_kwargs) { fn = _fn; return_value = _return_value; @@ -446,14 +476,18 @@ void CallObject::parse_args(zval *array) { zend_ulong num_key; ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(array), num_key, string_key, current) { + auto elem = php2py(current); if (!string_key) { - PyList_Append(arg_list, php2py(current)); + PyList_Append(arg_list, elem); } else { if (kwargs == nullptr) { kwargs = PyDict_New(); } - PyDict_SetItem(kwargs, string2py(string_key), php2py(current)); + auto dk = string2py(string_key); + PyDict_SetItem(kwargs, dk, elem); + Py_DECREF(dk); } + Py_DECREF(elem); (void) num_key; } ZEND_HASH_FOREACH_END(); @@ -526,6 +560,8 @@ void string2zval(PyObject *pv, zval *zv) { void tuple2argv(zval *argv, PyObject *args, ssize_t size, int begin) { Py_ssize_t i; for (i = begin; i < size; i++) { + // PyTuple_GetItem() + // Return value: Borrowed reference PyObject *arg = PyTuple_GetItem(args, i); if (arg == NULL) { PyErr_SetString(PyExc_TypeError, "wrong parameter"); diff --git a/src/php/dict.cc b/src/php/dict.cc index bf4df9d..9244fd7 100644 --- a/src/php/dict.cc +++ b/src/php/dict.cc @@ -90,11 +90,9 @@ ZEND_METHOD(PyDict, offsetSet) { auto object = phpy_object_get_handle(ZEND_THIS); PyObject *pv = php2py(zv); PyObject *pk = php2py(zk); - ON_SCOPE_EXIT { - Py_DECREF(pv); - Py_DECREF(pk); - }; auto value = PyDict_SetItem(object, pk, pv); + Py_DECREF(pv); + Py_DECREF(pk); if (value < 0) { phpy::php::throw_error_if_occurred(); } diff --git a/src/php/list.cc b/src/php/list.cc index 8e7cc12..f8d7eb9 100644 --- a/src/php/list.cc +++ b/src/php/list.cc @@ -77,6 +77,8 @@ ZEND_METHOD(PyList, offsetGet) { zend_throw_error(NULL, "PyList: index[%ld] out of range", pk); return; } + // PyList_GetItem() + // Return value: Borrowed reference auto value = PyList_GetItem(object, pk); if (value != NULL) { py2php(value, return_value); @@ -94,16 +96,16 @@ ZEND_METHOD(PyList, offsetSet) { auto object = phpy_object_get_handle(ZEND_THIS); PyObject *pv = php2py(zv); - ON_SCOPE_EXIT { - Py_DECREF(pv); - }; int result; if (zk == NULL || ZVAL_IS_NULL(zk)) { result = PyList_Append(object, pv); } else { Py_INCREF(pv); + // PyList_SetItem() + // Not increase reference count of the value result = PyList_SetItem(object, zval_get_long(zk), pv); } + Py_DECREF(pv); if (result < 0) { phpy::php::throw_error_if_occurred(); } diff --git a/src/php/object.cc b/src/php/object.cc index c48f483..6bea5ab 100644 --- a/src/php/object.cc +++ b/src/php/object.cc @@ -332,15 +332,18 @@ ZEND_METHOD(PyObject, count) { ZEND_METHOD(PyObject, offsetGet) { auto pk = arg_1(INTERNAL_FUNCTION_PARAM_PASSTHRU); auto object = phpy_object_get_handle(ZEND_THIS); - ON_SCOPE_EXIT { - Py_DECREF(pk); - }; + /** + * PyObject_GetItem() + * Return value: New reference + */ auto value = PyObject_GetItem(object, pk); + Py_DECREF(pk); if (value == NULL) { phpy::php::throw_error_if_occurred(); return; } py2php(value, return_value); + Py_DECREF(value); } ZEND_METHOD(PyObject, offsetSet) { @@ -355,11 +358,13 @@ ZEND_METHOD(PyObject, offsetSet) { auto object = phpy_object_get_handle(ZEND_THIS); PyObject *pv = php2py(zv); PyObject *pk = php2py(zk); - ON_SCOPE_EXIT { - Py_DECREF(pv); - Py_DECREF(pk); - }; + /** + * PyObject_SetItem() + * Increase reference count of the value + */ auto value = PyObject_SetItem(object, pk, pv); + Py_DECREF(pv); + Py_DECREF(pk); if (value < 0) { phpy::php::throw_error_if_occurred(); } diff --git a/src/php/tuple.cc b/src/php/tuple.cc index 7a2dc04..c929410 100644 --- a/src/php/tuple.cc +++ b/src/php/tuple.cc @@ -78,6 +78,8 @@ ZEND_METHOD(PyTuple, offsetGet) { zend_throw_error(NULL, "PyTuple: index[%ld] out of range", pk); return; } + // PyTuple_GetItem() + // Return value: Borrowed reference auto value = PyTuple_GetItem(object, pk); if (value != NULL) { py2php(value, return_value); diff --git a/tools/mem_leak.php b/tools/mem_leak.php new file mode 100644 index 0000000..3dfec01 --- /dev/null +++ b/tools/mem_leak.php @@ -0,0 +1,74 @@ +