Ref count for dict item should be Py_DECREF

PyDict_SetItems doesn't steal the reference count so that the reference
count of each item in the dict should be decremented via Py_DECREF
*before* returning the dict to Python VM. Otherwise, those items can be
leaked which can never be collected as garbage.

The evicence for memory leak is avaialble for using debug mode python VM like:
https://gist.github.com/bloodeagle40234/f4c0cd267e085cc6224ffdc1b1822631

Plus, this patch add one unit test to check the memory increasement for
many call of get_segment_info.

Closes-Bug: #1604335

Change-Id: I6780efb9718017d296606f3c7e60684910584a1a

2 files changed, 78 insertions, 8 deletions

diff --git a/src/c/pyeclib_c/pyeclib_c.c b/src/c/pyeclib_c/pyeclib_c.c
index 466fa40..d6b546f 100644
--- a/src/c/pyeclib_c/pyeclib_c.c
+++ b/src/c/pyeclib_c/pyeclib_c.c
@@ -354,6 +354,19 @@ pyeclib_c_get_segment_info(PyObject *self, PyObject *args)
   PyObject *pyeclib_obj_handle = NULL;
   pyeclib_t *pyeclib_handle = NULL;
   PyObject *ret_dict = NULL;               /* python dictionary to return */
+
+  // Prepare variables for return dict to cleanup on exit
+  PyObject *segment_size_key = NULL;
+  PyObject *segment_size_value = NULL;
+  PyObject *last_segment_size_key = NULL;
+  PyObject *last_segment_size_value = NULL;
+  PyObject *fragment_size_key = NULL;
+  PyObject *fragment_size_value = NULL;
+  PyObject *last_fragment_size_key = NULL;
+  PyObject *last_fragment_size_value = NULL;
+  PyObject *num_segments_key = NULL;
+  PyObject *num_segments_value = NULL;
+
   int data_len;                            /* data length from user in bytes */
   int segment_size, last_segment_size;     /* segment sizes in bytes */
   int num_segments;                        /* total number of segments */
@@ -451,16 +464,49 @@ pyeclib_c_get_segment_info(PyObject *self, PyObject *args)
   /* Create and return the python dictionary of segment info */
   ret_dict = PyDict_New();
   if (NULL == ret_dict) {
-    pyeclib_c_seterr(-ENOMEM, "pyeclib_c_get_segment_info ERROR: ");
+    goto 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));
+    if((segment_size_key = PyString_FromString("segment_size\0")) == NULL ||
+       (segment_size_value = PyInt_FromLong(segment_size)) == NULL ||
+       PyDict_SetItem(ret_dict, segment_size_key, segment_size_value)) goto error;
+
+    if((last_segment_size_key = PyString_FromString("last_segment_size\0")) == NULL ||
+       (last_segment_size_value = PyInt_FromLong(last_segment_size)) == NULL ||
+       PyDict_SetItem(ret_dict, last_segment_size_key, last_segment_size_value)) goto error;
+
+    if((fragment_size_key = PyString_FromString("fragment_size\0")) == NULL ||
+       (fragment_size_value = PyInt_FromLong(fragment_size)) == NULL ||
+       PyDict_SetItem(ret_dict, fragment_size_key, fragment_size_value)) goto error;
+
+    if((last_fragment_size_key = PyString_FromString("last_fragment_size\0")) == NULL ||
+       (last_fragment_size_value = PyInt_FromLong(last_fragment_size)) == NULL ||
+        PyDict_SetItem(ret_dict, last_fragment_size_key, last_fragment_size_value)) goto error;
+
+    if((num_segments_key = PyString_FromString("num_segments\0")) == NULL ||
+       (num_segments_value = PyInt_FromLong(num_segments)) == NULL ||
+       PyDict_SetItem(ret_dict, num_segments_key, num_segments_value)) goto error;
   }
-  
-  return ret_dict;
+
+exit:
+    Py_XDECREF(segment_size_key);
+    Py_XDECREF(segment_size_value);
+    Py_XDECREF(last_segment_size_key);
+    Py_XDECREF(last_segment_size_value);
+    Py_XDECREF(fragment_size_key);
+    Py_XDECREF(fragment_size_value);
+    Py_XDECREF(last_fragment_size_key);
+    Py_XDECREF(last_fragment_size_value);
+    Py_XDECREF(num_segments_key);
+    Py_XDECREF(num_segments_value);
+    return ret_dict;
+
+error:
+    // To prevent unexpected call, this is placed after return call
+    pyeclib_c_seterr(-ENOMEM, "pyeclib_c_get_segment_info ERROR: ");
+    Py_XDECREF(ret_dict);
+    ret_dict = NULL;
+    goto exit;
+
 }
 
 
diff --git a/test/test_pyeclib_api.py b/test/test_pyeclib_api.py
index a8ee926..4eb6bcf 100644
--- a/test/test_pyeclib_api.py
+++ b/test/test_pyeclib_api.py
@@ -38,6 +38,8 @@ from pyeclib.ec_iface import PyECLib_EC_Types
 from pyeclib.ec_iface import ALL_EC_TYPES
 from pyeclib.ec_iface import VALID_EC_TYPES
 from pyeclib.ec_iface import LIBERASURECODE_VERSION
+import resource
+
 
 if sys.version < '3':
     def b2i(b):
@@ -610,6 +612,28 @@ class TestPyECLibDriver(unittest.TestCase):
                 self.assertTrue(
                     pyeclib_drivers[0].min_parity_fragments_needed() == 1)
 
+    def test_get_segment_info_memory_usage(self):
+        for ec_driver in self.get_pyeclib_testspec():
+            self._test_get_segment_info_memory_usage(ec_driver)
+
+    def _test_get_segment_info_memory_usage(self, ec_driver):
+        # 1. Preapre the expected memory allocation
+        info = ec_driver.get_segment_info(1024*1024, 1024*1024)
+        info = None
+        loop_range = range(1000)
+
+        # 2. Get current memory usage
+        usage = resource.getrusage(resource.RUSAGE_SELF)[2]
+
+        # 3. Loop to call get_segment_info
+        for x in loop_range:
+            ec_driver.get_segment_info(1024*1024, 1024*1024)
+
+        # 4. memory usage shoudln't be increased
+        self.assertEqual(usage, resource.getrusage(resource.RUSAGE_SELF)[2],
+                         'Memory usage is increased unexpectedly %s - %s' %
+                         (usage, resource.getrusage(resource.RUSAGE_SELF)[2]))
+
 
 if __name__ == '__main__':
     unittest.main()