Should I use Py_INCREF for any of my PyObjects in this block? Also am I Py_DECREFing my objects correctly?

Question

Whenever I call this function, memory usage is increases a lot per call, so I think there is some memory leak here.

PyObject *pScript, *pModule, *pFunc, *pValue;
PyObject *pArgs = NULL;
long ret = 1;

// Initialize python, set system path and load the module
pScript = SetPyObjectString(PYTHON_SCRIPT_NAME);
PyRun_SimpleString("import sys");
PyRun_SimpleString("sys.path.append('"PYTHON_SCRIPT_PATH"')");
pModule = PyImport_Import(pScript);
Py_XDECREF(pScript);

if (pModule != NULL) {
    // Get function object from python module
    pFunc = PyObject_GetAttrString(pModule, operation.c_str());

    if (pFunc && PyCallable_Check(pFunc)) {
        // Create argument(s) as Python tuples
        if (operation == UPDATE_KEY) {
            // If operation is Update key, create two arguments - key and value
            pArgs = PyTuple_New(2);
        }
        else {
            pArgs = PyTuple_New(1);
        }

        pValue = SetPyObjectString(key.c_str());
        // Set argument(s) with key/value strings
        PyTuple_SetItem(pArgs, 0, pValue);
        if (operation == UPDATE_KEY) {
            // If operation is Update key, set two arguments - key and value
            pValue = SetPyObjectString(value.c_str());
            PyTuple_SetItem(pArgs, 1, pValue);
        }

        // Call the function using function object and arguments
        pValue = PyObject_CallObject(pFunc, pArgs);
        Py_XDECREF(pArgs);

        if (pValue != NULL) {
            // Parse the return values
            ret = PyLong_AsLong(PyList_GetItem(pValue, 0));
            value = GetPyObjectString(PyList_GetItem(pValue, 1));
        }
        else {
            ERROR("Function call to %s failed", operation.c_str());
        }

        Py_XDECREF(pValue);
        Py_XDECREF(pFunc);
    }
    else {
        ERROR("Cannot find function in python module");
    }
    Py_XDECREF(pModule);
}
else {
    ERROR("Failed to load python module");
}

I am leaking some memory when this C++ snippet in my code calls the python script and I want to know why. I think I am doing something wrong with my Py_DECREFs. Any help would be much appreciated.

Answer 1

I spotted one missing decref from a quick glance:

---

pFunc = PyObject_GetAttrString(pModule, operation.c_str());

if (pFunc && PyCallable_Check(pFunc)) {
    // ...
    Py_XDECREF(pFunc);
}

This will leak any non-callable attribute matching operation.

---

The two reassignments of pValue… I think that's OK, because PyTuple_SetItem steals the reference to each of the original values.

---

For this line that you asked about:

value = GetPyObjectString(PyList_GetItem(pValue, 1));

The PyList_GetItem returns a borrowed reference, so the fact that you don't decref it is correct.

But I don't see the declaration for value anywhere, or GetPyObjectString, so I have no idea what that part is doing. Maybe it's just getting a borrowed buffer out of a PyUnicodeObject * and copying it into some C++ wstring or UTF-32 string type, or maybe it's leaking a Python object or a copied buffer, or returning a raw C buffer that you just leak here, or… who knows?

---

But I certainly wouldn't trust that some guy on the internet found all of them on a quick scan. Learn to use a memory debugger.

Or: You're using C++. RAII is almost the whole point of using C++—in other words, instead of using raw PyObject * values, you can use a smart pointer that decrefs things for you automatically. Or, even better, use a ready-made library like PyCXX.