Comment 3 for bug 1604335

Thanks, Kota! Let me know when you add push the fix and I can review it.

-kevin

On Wed, Jul 20, 2016 at 2:11 AM, Kota Tsuyuzaki <
<email address hidden>> wrote:

> Add Swift as affected project because this can cause memory leak in
> running Swift (with EC policy)
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1604335
>
> Title:
> get_segment_info leaks reference count for items
>
> Status in PyECLib:
> New
> Status in OpenStack Object Storage (swift):
> New
>
> Bug 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
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/pyeclib/+bug/1604335/+subscriptions
>