Memory Leak in Python SWIG Messages

[rbenefo]

There seems to be a memory leak when writing Python SWIG messages -- e.g., on OS-X, I'm seeing a steady increase in memory usage when the below code is run:

while True:
    att_ref_out_payload = messaging.AttRefMsgPayload()
    arr = [1., 2., 3]
    att_ref_out_payload.sigma_RN = arr

If I'm understanding how SWIG works correctly, this seems to be driven by the PySequence_GetItem call here: link (code snippet below):

%typemap(in) type [ANY] (type temp[$1_dim0]) {
    int i;
    void *blankPtr = 0 ;
    int resOut = 0 ;
    if (!PySequence_Check($input)) {
        PyErr_SetString(PyExc_ValueError,"Expected a sequence");
        return NULL;
    }
    if (PySequence_Length($input) > $1_dim0) {
        printf("Value: %d\n", $1_dim0);
        PyErr_SetString(PyExc_ValueError,"Size mismatch. Expected $1_dim0 elements");
        return NULL;
    }
    memset(temp, 0x0, $1_dim0*sizeof(type));
    for (i = 0; i < PySequence_Length($input); i++) {
        PyObject *o = PySequence_GetItem($input,i);
        if (PyNumber_Check(o)) {
            temp[i] = (type)PyFloat_AsDouble(o);
        } else {
            resOut = SWIG_ConvertPtr(o, &blankPtr,$1_descriptor, 0 |  0 );
            if (!SWIG_IsOK(resOut)) {
                SWIG_exception_fail(SWIG_ArgError(resOut), "Could not convert that type into a pointer for some reason.  This is an ugly SWIG failure.  Good luck.\n");
                return NULL;
            }
            memcpy(&(temp[i]), blankPtr, sizeof(type));
        }
    }
    $1 = temp;
}

PySequence_GetItem, according to its docs, generates a new reference -- but in this code, we're never freeing that reference even after we do thememcpy. I think this should be fixed by adding a couple Py_DECREF(o) statements (this fixes the memory leak on my hardware):

%typemap(in) type [ANY] (type temp[$1_dim0]) {
    int i;
    void *blankPtr = 0 ;
    int resOut = 0 ;
    if (!PySequence_Check($input)) {
        PyErr_SetString(PyExc_ValueError,"Expected a sequence");
        return NULL;
    }
    if (PySequence_Length($input) > $1_dim0) {
        printf("Value: %d\n", $1_dim0);
        PyErr_SetString(PyExc_ValueError,"Size mismatch. Expected $1_dim0 elements");
        return NULL;
    }
    memset(temp, 0x0, $1_dim0*sizeof(type));
    for (i = 0; i < PySequence_Length($input); i++) {
        PyObject *o = PySequence_GetItem($input,i);
        if (PyNumber_Check(o)) {
            temp[i] = (type)PyFloat_AsDouble(o);
        } else {
            resOut = SWIG_ConvertPtr(o, &blankPtr,$1_descriptor, 0 |  0 );
            if (!SWIG_IsOK(resOut)) {
                SWIG_exception_fail(SWIG_ArgError(resOut), "Could not convert that type into a pointer for some reason.  This is an ugly SWIG failure.  Good luck.\n");
                Py_DECREF(o); // Decrement the ref count here in case of  #error
                return NULL;
            }
            memcpy(&(temp[i]), blankPtr, sizeof(type));
        }
    }
    Py_DECREF(o);
    $1 = temp;
}

[schaubh]

Thanks for sharing this issue. We have been trying to resolve a range of swig related memory leaks. @juan-g-bonilla or @patkenneally , any thoughts on this suggestion?

[juan-g-bonilla]

@rbenefo is right, PySequence_GetItem is increasing the reference count, so we need to manually decrease it. Something similar was causing leaks in swig_eigen. I can't vouch for this specific implementation, I would have to dig to make sure we are getting the references right with all flow paths, but I can believe that there is a leak here and is being caused by PySequence_GetItem. For a PR, I would recommend checking all instances of PySequence_GetItem in that file, not just in %typemap(in) type [ANY] (type temp[$1_dim0]). We should be very careful testing this because if we decrease the count when we shouldn´t we could be freeing memory twice or accessing it after freeing it, which is not only a bug, but possibly a hard bug to find.

[schaubh]

Thanks for your feedback on this @juan-g-bonilla .

[rbenefo]

Thanks @juan-g-bonilla and @schaubh. Happy to submit a PR on this; let me know how best I can help.

[schaubh]

@rbenefo , I don't have the bandwidth to look into this right now. If you want test this on a branch and create a PR when you think you have a solution, that would be helpful.