Thread Subject:

mexCallMATLAB crashes in loops?

Hello everyone, I'm working on a mex code that using mexCallMATLAB several times in a loop. The input is supposed to be a structure matrix and the matlab function called in the mex code dealing with a single structure data.

The matlab fuction called in mex is like this:
<simple.m>
function y=simple(x)
y=zeros(3,1);
y(1)=x.field_2+1;
y(2)=2;
y(3)=3;
</simple.m>

The mex code I wrote:

#include "mex.h"
#include "math.h"
#include <stdlib.h>

void mexFunction(int nlhs, mxArray *plhs[],int nrhs, mxArray *prhs[])
{
    const char **fnames;
    const int *coutsize;
    int ifield, nfields, ninputs;
    double *loopout;
    double *cout;
    double n1,n2;
    
mwSize structdims[2]={1,1};
mwSize outputdims[2];
mwIndex istruct;
mxArray *incall[1];
    mxArray *tmp;
    mxArray *mout[1];


    /* Check for proper number of arguments */
    if (nrhs != 1) {
mexErrMsgTxt("One input argument required.");
    } else if (nlhs > 1) {
mexErrMsgTxt("Too many output arguments.");
} else if ((mxIsStruct(prhs[0]) != 1)){
mexErrMsgTxt("Input must be structure");
}
    
/* Create single struct array */
nfields = mxGetNumberOfFields(prhs[0]);
    ninputs = mxGetN(prhs[0]);
fnames = (const char **)mxCalloc(nfields, sizeof(fnames));
for(ifield=0; ifield<nfields; ifield++){
        fnames[ifield] = mxGetFieldNameByNumber(prhs[0],ifield);}
incall[0] = mxCreateStructArray(2,structdims , nfields, fnames);
mxFree((void *)fnames);
    
    for(istruct=0;istruct<ninputs;istruct++){
            
        for(ifield=0; ifield<nfields; ifield++){
            tmp=mxDuplicateArray(mxGetFieldByNumber(prhs[0],istruct,ifield));
            mxSetFieldByNumber(incall[0], istruct, ifield, tmp);
        }
            
        mexCallMATLAB(1, mout, 1, incall, "simple");
            
        cout = mxGetPr(mout[0]);
        coutsize=mxGetDimensions(mout[0]);
            
        if(istruct==0){
            mexPrintf("Initializing...\n");
            n1 = coutsize[0];
            n2 = 2 * n1;
                
            /* Create Output Value */
            outputdims[0] = n2;
            outputdims[1] = ninputs;
            plhs[0]=mxCreateNumericArray(2, outputdims, mxDOUBLE_CLASS, mxREAL);
            loopout = mxGetPr(plhs[0]);
        }
            
        for(int it=0;it<coutsize[0];it++){
            loopout[(int)istruct*(int)n2+it] = 2*cout[it];
        }
        
        mexPrintf("now:%d\n",istruct);
    }
    
//Clean up
     mxDestroyArray(incall[0]);
     mxDestroyArray(mout[0]);
     mxDestroyArray(tmp);
}

The input structure matrix I used to test the code was:
input(1).field_1='a1';
input(1).field_2=1.11;
input(1).field_3=[1,1,1];

input(2).field_1='a2';
input(2).field_2=2.22;
input(2).field_3=[2,2,2];

input(3).field_1='a3';
input(3).field_2=3.33;
input(3).field_3=[3,3,3];

When there is no loop, everything worked well. As long as I add the loop in, matlab crashes. There might be errors around dimensions or output settings that I'm not able to figure out. Could you give me any suggestions?

Best regards

"Yi " <dora.zhuyi@gmail.com> wrote in message <kbt4p3$n00$1@newscl01ah.mathworks.com>...

>
> //Clean up
> mxDestroyArray(incall[0]);
> mxDestroyArray(mout[0]);
> mxDestroyArray(tmp);

I haven't run your code, just take a quick look, but it seems wrong to destroy TMP separately, since it belongs to INCALL[0] and already destroyed when mxDestroyArray(incall[0]) is invoked.

Bruno

"Bruno Luong" <b.luong@fogale.findmycountry> wrote in message <kbu49s$3g8$1@newscl01ah.mathworks.com>...
> "Yi " <dora.zhuyi@gmail.com> wrote in message <kbt4p3$n00$1@newscl01ah.mathworks.com>...
>
> >
> > //Clean up
> > mxDestroyArray(incall[0]);
> > mxDestroyArray(mout[0]);
> > mxDestroyArray(tmp);
>
> I haven't run your code, just take a quick look, but it seems wrong to destroy TMP separately, since it belongs to INCALL[0] and already destroyed when mxDestroyArray(incall[0]) is invoked.
>
> Bruno

Thanks for your reply, Bruno. In fact, I have several trys with and without these mxDestroyArray lines and it turns out all the same. Sometimes I can see the returned variable in matlab but with no dimension information. And When I try to access it, the MATLAB crashes.

"Yi " <dora.zhuyi@gmail.com> wrote in message <kbv5ko$it4$1@newscl01ah.mathworks.com>...
> "Bruno Luong" <b.luong@fogale.findmycountry> wrote in message <kbu49s$3g8$1@newscl01ah.mathworks.com>...
> > "Yi " <dora.zhuyi@gmail.com> wrote in message <kbt4p3$n00$1@newscl01ah.mathworks.com>...
> >
> > >
> > > //Clean up
> > > mxDestroyArray(incall[0]);
> > > mxDestroyArray(mout[0]);
> > > mxDestroyArray(tmp);
> >
> > I haven't run your code, just take a quick look, but it seems wrong to destroy TMP separately, since it belongs to INCALL[0] and already destroyed when mxDestroyArray(incall[0]) is invoked.
> >
> > Bruno
>
> Thanks for your reply, Bruno. In fact, I have several trys with and without these mxDestroyArray lines and it turns out all the same. Sometimes I can see the returned variable in matlab but with no dimension information. And When I try to access it, the MATLAB crashes.

First, as Bruno says, get rid of the mxDestroyArray(tmp) line. The tmp variable should *not* be destroyed separately since it is part of another variable already. This in and of itself will lead to a crash.

Secondly, it appears that you only allocate a 1x1 struct in your code. E.g.,

mwSize structdims[2]={1,1};
    :
incall[0] = mxCreateStructArray(2,structdims , nfields, fnames);

So there is only one element in the incall[0] variable. However, you have a loop like this:

ninputs = mxGetN(prhs[0]);
    :
    for(istruct=0;istruct<ninputs;istruct++){
        for(ifield=0; ifield<nfields; ifield++){
            tmp=mxDuplicateArray(mxGetFieldByNumber(prhs[0],istruct,ifield));
            mxSetFieldByNumber(incall[0], istruct, ifield, tmp);
        }

So if istruct is 1 or bigger, the code will bomb since there are no elements allocated beyond the 0 element (i.e., your 1x1 allocation). I think you have a disconnect between how you are allocating the size of incall[0] and your input. I suspect you want something like this instead:

ninputs = mxGetNumberOfElements(prhs[0]);
    :
structdims[1]=ninputs;
    :
incall[0] = mxCreateStructArray(2,structdims , nfields, fnames);
    :
  etc

James Tursa

Thank you, James. Actually I only need one of the structure element each time for the function input, and I was messing up when setting the field values for the incall.

I fixed the code in this way:

    :
    for(istruct=0;istruct<ninputs;istruct++){
            
        for(ifield=0; ifield<nfields; ifield++){
            tmp=mxDuplicateArray(mxGetFieldByNumber(prhs[0],istruct,ifield));
            mxSetFieldByNumber(incall[0], 0, ifield, tmp);
        }
     :
     mxDestroyArray(mout[0]);
    }

I got some ideas from this thread:
http://www.mathworks.co.kr/matlabcentral/newsreader/view_thread/280511

So I cleaned the mout[0] in each loop and cleaned the incall[0] in the end. The code works for now yet I still have a question:

Is it necessary to clean the mout[0] in the loop? Can I clean it up in the end of the code (with incall[0])?

Thank you again for your helpful replies :)

"Yi " <dora.zhuyi@gmail.com> wrote in message <kc2cor$he8$1@newscl01ah.mathworks.com>...
>
> So I cleaned the mout[0] in each loop and cleaned the incall[0] in the end. The code works for now yet I still have a question:
>
> Is it necessary to clean the mout[0] in the loop? Can I clean it up in the end of the code (with incall[0])?

Each mout[0] that gets created inside your loop is tagged as a temporary variable that gets put on the garbage collection list. Every time you call mexCallMATLAB to generate another mout[0] you lose the pointer to the old mout[0] variable (i.e., the current mout[0] value gets wiped out and overwritten by the new value) and thus you have created a temporary memory leak. However, once your mex function returns control back to the calling function (i.e., MATLAB in this case) all of the variables on the garbage collection list get automatically destroyed by MATLAB, so your memory leaks get fixed. This is why the memory leaks you created inside the loop are temporary. That being said, it is good programming practice not to generate memory leaks like this in the first place, so I would advise destroying the mout[0] variables at the end of each loop iteration (and concurrently get rid of the
one mxDestroyArray(mout[0]) at the end).

James Tursa

"Yi " <dora.zhuyi@gmail.com> wrote in message <kc2cor$he8$1@newscl01ah.mathworks.com>...
> Thank you, James. Actually I only need one of the structure element each time for the function input, and I was messing up when setting the field values for the incall.
>
> I fixed the code in this way:
>
> :
> for(istruct=0;istruct<ninputs;istruct++){
>
> for(ifield=0; ifield<nfields; ifield++){
> tmp=mxDuplicateArray(mxGetFieldByNumber(prhs[0],istruct,ifield));
> mxSetFieldByNumber(incall[0], 0, ifield, tmp);
> }
> :
> mxDestroyArray(mout[0]);
> }

I should add that your shown method above also creates memory leaks since mxSetFieldByNumber does *not* destroy any current stuff before replacing the field pointer with tmp. To avoid a memory leak you would have to do something like this instead:

         for(ifield=0; ifield<nfields; ifield++){
             tmp=mxGetFieldByNumber(incall[0],0,ifield);
             if( tmp ) {
                 mxDestroyArray(tmp);
                 mxSetFieldByNumber(incall[0], 0, ifield, NULL);
             }
             tmp=mxDuplicateArray(mxGetFieldByNumber(prhs[0],istruct,ifield));
             mxSetFieldByNumber(incall[0], 0, ifield, tmp);
         }

The reason for setting the field to NULL just after destroying tmp is so that your code will not bomb in the event that the mxDuplicateArray call fails (e.g., out of memory). Technically, after the mxDestroyArray(tmp) call the incall[0] variable is in an invalid state since one of its elements points to an invalid memory location. It is best to fix that right away before any downstream code has a chance to error-out back to MATLAB.

James Tursa

"Yi " <dora.zhuyi@gmail.com> wrote in message <kc2cor$he8$1@newscl01ah.mathworks.com>...
> :
> for(istruct=0;istruct<ninputs;istruct++){
>
> for(ifield=0; ifield<nfields; ifield++){
> tmp=mxDuplicateArray(mxGetFieldByNumber(prhs[0],istruct,ifield));
> mxSetFieldByNumber(incall[0], 0, ifield, tmp);
> }

I should further add that if you are going to be working with large variables, there are better functions to use for getting read-only copies of stuff into struct or cell elements ... they are the undocumented functions mxCreateSharedDataCopy and mxCreateReference. Their use would allow you to create a struct or cell array that contains only parts of your original struct or cell without actually copying the data (i.e., you just copy the variable pointers over to the new array in a special way using the afore-mentioned undocumented functions).

James Tursa

"James Tursa" wrote in message <kc2g3l$sb4$1@newscl01ah.mathworks.com>...
> "Yi " <dora.zhuyi@gmail.com> wrote in message
>
> I should further add that if you are going to be working with large variables, there are better functions to use for getting read-only copies of stuff into struct or cell elements ... they are the undocumented functions mxCreateSharedDataCopy and mxCreateReference.

James, can you please post a calling syntax of mxCreateSharedDataCopy and mxCreateReference ? Thanks.

Bruno

"Bruno Luong" <b.luong@fogale.findmycountry> wrote in message <kc3aot$ogg$1@newscl01ah.mathworks.com>...
> "James Tursa" wrote in message <kc2g3l$sb4$1@newscl01ah.mathworks.com>...
> > "Yi " <dora.zhuyi@gmail.com> wrote in message
> >
> > I should further add that if you are going to be working with large variables, there are better functions to use for getting read-only copies of stuff into struct or cell elements ... they are the undocumented functions mxCreateSharedDataCopy and mxCreateReference.
>
> James, can you please post a calling syntax of mxCreateSharedDataCopy and mxCreateReference ? Thanks.
>
> Bruno

mxArray *mxCreateSharedDataCopy(mxArray *mx);

Returns a pointer to a shared data copy of the input mx. The resulting mxArray will be of type TEMPORARY (4) (i.e., on the garbage collection list).

mxArray *mxCreateReference(mxArray *mx);

Returns the value mx after bumping up the reference count of mx by 1. The intent is to use this function on a variable that is already a sub-element (i.e., part of a struct or cell array) and immediately place the result into another struct field or cell array element so that it gets destroyed the appropriate number of times when variables get cleared. I.e., the first time it gets destroyed the reference count simply gets bumped down by 1 (nothing actually gets destroyed). The last time it gets destroyed (after the reference count has been bumped down to 0) the header and data areas are actually destroyed (freed).

James Tursa

Thank you James. Very useful information.

Bruno

Thank you, James.