Optimize reference count code, fix memory leak

swoole · Jun 12, 2024 · dd161ca · dd161ca
1 parent 3446a88
commit dd161ca
Show file tree

Hide file tree

Showing 7 changed files with 137 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,74 @@
+<?php
+
+ini_set('memory_limit', '1G');
+
+$gc = PyCore::import('gc');
+
+class a
+{
+ public function __construct(
+ public array $a
+ )
+ {
+ }
+}
+
+// 内存不会持续增长
+function test()
+{
+ $arr = [];
+
+ for ($i = 0; $i < 100000; $i++) {
+ $arr[] = $i;
+ }
+
+ $a = new a($arr);
+}
+
+// 内存会持续增长
+function test1()
+{
+ $arr = [];
+ for ($i = 0; $i < 100000; $i++) {
+ $arr[] = $i;
+ }
+ $a = new PyTuple($arr);
+ $a[1] = uniqid();
+ echo $a[1];
+
+ $a[] = uniqid();
+
+ $a = null;
+ unset($a);
+}
+
+function test2()
+{
+ $arr = [];
+ for ($i = 0; $i < 10000; $i++) {
+ $arr['key-' . $i] = $i;
+ }
+ $a = new PyDict($arr);
+ $a = null;
+ unset($a);
+}
+
+function test_tuple()
+{
+ $arr = [];
+ for ($i = 0; $i < 100000; $i++) {
+ $arr[] = $i;
+ }
+ $a = new PyTuple($arr);
+ $a = null;
+ unset($a);
+}
+
+$n = 1000;
+
+
+while ($n--) {
+ test_tuple();
+ sleep(1);
+ var_dump(memory_get_usage());
+}