XS Memory leak in this code?

Question

Unable to find where the memory leak is happening in this code.

Basically I want to write a XS wrapper for a C-function which returns a two-dimensional array.

C-function:

int CW_returnArray(double** arrayDouble, int* count)
{
    int number = 10;
    int index, index1;
    for(index = 0; index < number; index++)
    {
        for(index1 = 0; index1 < 10000; index1++)
        {
            arrayDouble[index][index1] = 12.51;
        }

        count[index] = 10000; 
    }
    return number;
}

array -> output param to hold the two dimensional array
count -> output param to hold the number of element in each 1D array

XS wrapper:

void
returnArray()
    PPCODE:
    {
    /** variable declaration **/
    double** array;
    int i = 0, j=0,  status;
    int* count;
    int totalArrays;

    SV** SVArrays;     // to hold the references of 1D arrays
    SV** SVtempArray;  // temporary array to hold the elements of 1D array

    /** allocate memory for C-type variables **/
    New(0, array, 10, double*);

    for(i = 0; i<10;i++)
    {
        New(0, array[i], 10000, double);
    }

    New(0, count, 10, int);

    /** call C function **/
    status = CW_returnArray(array, count); 

    /** check the status and retrieve the array to store it in stack **/
    if(status > 0)
    {
        totalArrays = status;

        New(0, SVArrays, totalArrays, SV*);
        for(i = 0; i<totalArrays; i++)
        {
            /** allocate memory for temporary SV array **/
            New(0, SVtempArray, count[i], SV*);
            for(j = 0; j<count[i]; j++)
            {
                SVtempArray[j] = newSVnv(array[i][j]);
            }

            /** Make an array (AV) out of temporary SV array and store the reference in SVArrays **/
            SVArrays[i] = newRV_noinc((SV*) av_make(count[i], SVtempArray));

            /** free the memory allocated for temp SV array **/
            for(j = 0; j<count[i]; j++)
            {
                sv_free(SVtempArray[j]);
            }               

            Safefree(SVtempArray); SVtempArray = NULL;
        }
    }
    else
    {
        totalArrays = 0;

    }

    /** push the return values to stack **/
    EXTEND(SP, 2);
    PUSHs(sv_2mortal(newSViv(status)));
    PUSHs(sv_2mortal(newRV_noinc((SV*) av_make(totalArrays, SVArrays))));

    /** clean up allocated memory for SV "array of array" , if needed **/
    if(totalArrays > 0)
    {
        Safefree(SVArrays); SVArrays = NULL;
    }

    /** clean up allocated memory for C-type variables **/
    for(i = 0; i<10;i++)
    {
        Safefree(array[i]);
    }       
    Safefree(array); array = NULL;
    Safefree(count); count = NULL;
}

An "array of array" is returned from XS.

testing in Perl script:

for(1..100)
{
    my ($status, $arrayref) = returnArray();
    undef $status;
    $arrayref = [];
    system('pause');
}

Every time the function returnArray() is called, the Commit size of Perl process is increasing. But I would expect that the $arrayref variable should be garbage collected every time and the Memory usage should not increase.

I hope, I am freeing all the allocated memory in XS. But still there is a memory leak. What is wrong with this XS code for memory leak?

Argumentum

You don't deallocate any of the scalars created using newSVnv. Instead of copying all those SVs using av_make, you should assign create an populate the array yourself. newAV+av_extend+av_fetch – Sternwheeler

But what about this code? /** free the memory allocated for temp SV array **/ for(j = 0; j<count[i]; j++) { sv_free(SVtempArray[j]); }. Won't the sv_free() call deallocate the SVs created using newSVnv()? – Argumentum

Never call sv_free. Use SvREFCNT_dec. – Sternwheeler

Answer 1

Well, the pattern of "create a template array, do av_make(), then free the template" is not very good -- you'd be much better by simply creating your array with newAV(), av_extend()ing it to the right size, and then doing av_store(newSVnv(...)) for each element. That lets you avoid the intermediate SVtempArray allocations entirely.

However, that's not what you asked about. I think your problem is that you Safefree(SVArrays) without first sv_free()ing each element. Since av_make() duplicates the contents of the source array, AFAICT you're leaking the reference created by

SVArrays[i] = newRV_noinc((SV*) av_make(count[i], SVtempArray));

You'll need to iterate over SVArrays and call sv_free() on each element before you Safefree(SVArrays).

Devorahdevore

thanks for pointing out the issue with this code. sv_free() was used at one place ( for(j = 0; j<count[i]; j++) { sv_free(SVtempArray[j]); } ) to free the SVs. But after pushing the final array into stack, the SVs were not sv_free()ed. – Argumentum