| 2016-07-20 13:31:36 |
Kota Tsuyuzaki |
description |
ECDriver.get_segment_info seems to leak reference counts for the return dict due to a lack of Py_DECREF for each PyObject. This seems to cause a memory leak for callers (in particular, swift can be affected because swift uses the call to calculate segment/fragment seize)
*Current* pyeclib does like here[1]:
ret_dict = PyDict_New();
if (NULL == ret_dict) {
pyeclib_c_seterr(-ENOMEM, "pyeclib_c_get_segment_info ERROR: ");
} else {
PyDict_SetItem(ret_dict, PyString_FromString("segment_size\0"), PyInt_FromLong(segment_size));
PyDict_SetItem(ret_dict, PyString_FromString("last_segment_size\0"), PyInt_FromLong(last_segment_size));
PyDict_SetItem(ret_dict, PyString_FromString("fragment_size\0"), PyInt_FromLong(fragment_size));
PyDict_SetItem(ret_dict, PyString_FromString("last_fragment_size\0"), PyInt_FromLong(last_fragment_size));
PyDict_SetItem(ret_dict, PyString_FromString("num_segments\0"), PyInt_FromLong(num_segments));
}
return ret_dict;
However, the docs for Python C-Extension says, sort of Python Object conversion (e.g. PyInt_FromLong) returns *a new reference* (i.e. increment the reference)[2]. And PyDict_SetItem seems to never steal the ownership for the reference unless the PyObject values throw the reference. Hence the reference counts of those objects included in the get_segment_info return value are never decremented, therefore they can not be collected as garbage.
An example to set items to the dict is available here [3]:
PyObject *key, *value;
Py_ssize_t pos = 0;
while (PyDict_Next(self->dict, &pos, &key, &value)) {
int i = PyInt_AS_LONG(value) + 1;
PyObject *o = PyInt_FromLong(i);
if (o == NULL)
return -1;
if (PyDict_SetItem(self->dict, key, o) < 0) {
Py_DECREF(o);
return -1;
}
Py_DECREF(o);
}
That needs Py_DECREF explicitly for each PyObject items of the dict.
With [4] code, I made sure the reference count increasing. (Note that it needs debug mode python built with --with-pydebug) And then, a patch I will attach the link for gerrit obviously prevent the ref count increasing.
1: https://github.com/openstack/pyeclib/blob/master/src/c/pyeclib_c/pyeclib_c.c#L433-L446
2: https://docs.python.org/2.7/c-api/int.html#c.PyInt_FromString
3: https://docs.python.org/2.7/c-api/dict.html#c.PyDict_Next
4: https://gist.github.com/bloodeagle40234/f4c0cd267e085cc6224ffdc1b1822631 |
ECDriver.get_segment_info seems to leak reference counts for the return dict due to a lack of Py_DECREF for each PyObject. This seems to cause a memory leak for callers (in particular, swift can be affected because swift uses the call to calculate segment/fragment seize)
*Current* pyeclib does like here[1]:
ret_dict = PyDict_New();
if (NULL == ret_dict) {
pyeclib_c_seterr(-ENOMEM, "pyeclib_c_get_segment_info ERROR: ");
} else {
PyDict_SetItem(ret_dict, PyString_FromString("segment_size\0"), PyInt_FromLong(segment_size));
PyDict_SetItem(ret_dict, PyString_FromString("last_segment_size\0"), PyInt_FromLong(last_segment_size));
PyDict_SetItem(ret_dict, PyString_FromString("fragment_size\0"), PyInt_FromLong(fragment_size));
PyDict_SetItem(ret_dict, PyString_FromString("last_fragment_size\0"), PyInt_FromLong(last_fragment_size));
PyDict_SetItem(ret_dict, PyString_FromString("num_segments\0"), PyInt_FromLong(num_segments));
}
return ret_dict;
However, the docs for Python C-Extension says, sort of Python Object conversion (e.g. PyInt_FromLong) returns *a new reference* (i.e. increment the reference)[2]. And PyDict_SetItem seems to never steal the ownership for the reference unless the PyObject values throw the reference. Hence the reference counts of those objects included in the get_segment_info return value are never decremented, therefore they can not be collected as garbage.
An example to set items to the dict is available here [3]:
PyObject *key, *value;
Py_ssize_t pos = 0;
while (PyDict_Next(self->dict, &pos, &key, &value)) {
int i = PyInt_AS_LONG(value) + 1;
PyObject *o = PyInt_FromLong(i);
if (o == NULL)
return -1;
if (PyDict_SetItem(self->dict, key, o) < 0) {
Py_DECREF(o);
return -1;
}
Py_DECREF(o);
}
That needs Py_DECREF explicitly for each PyObject items of the dict.
With [4] code, I made sure the reference count increasing. (Note that it needs debug mode python built with --with-pydebug) And then, a patch I will attach the link for gerrit obviously prevent the ref count increasing.
In Swift perspective, PUT/GET objects against to EC policy containers are affected.
GET objects are always affected since EC supported. PUT objects are affected only if the requests has content-length and affected versions have been since https://github.com/openstack/swift/commit/b5a243e75a8033988063d2c1c90ac373bc0050d2
1: https://github.com/openstack/pyeclib/blob/master/src/c/pyeclib_c/pyeclib_c.c#L433-L446
2: https://docs.python.org/2.7/c-api/int.html#c.PyInt_FromString
3: https://docs.python.org/2.7/c-api/dict.html#c.PyDict_Next
4: https://gist.github.com/bloodeagle40234/f4c0cd267e085cc6224ffdc1b1822631 |
|
