How to identify memory leaks in extensions?
Travis Oliphant
olipt at mayo.edu
Thu May 11 03:13:42 EDT 2000
>
> For the curious, the code is posted below if you'd like to offer
> specific rather than general advice. Either is appreciated!
I'll try to give some advice. Look at the reference section on
C-extension modules in the documentation. It contains information on what
needs to be DECREF'd. I wrote it last year and could probably do a better
job this year, but it does explain which commands increment the reference
count.
The short answer is, everything must be DECREF'd. Whenever you use a
PyArray_XXXXXXX construct that gives you back a PyArrayObject, you own a
reference. You must DECREF before leaving the subroutine or you have a
memory leak.
I'll point out what I find, after a quick perusal.
>
> Thanks in advance for any help you can offer,
> Tom Loredo
>
> /*------------------------------------------------------------------------------
> * Calculate the model metric & related quantities needed for the marginal
> * distribution for the nonlinear parameters. Return them in a tuple:
> *
> * (metric, L, rdet, P, A, S)
> *
> * metric = model metric (MxM matrix)
> * L = Cholesky factorization of metric (lower triangular MxM)
> * rdet = Sqrt of determinant of the metric
> * P = Projections of data on models (M vector)
> * A = Amplitude estimates (M vector)
> * S = Sufficient statistic
> *
> * The Cholesky algorithms are adapted from *Numerical Recipes*.
> ------------------------------------------------------------------------------*/
> static char vba_metricAnalysis_doc[] =
> "metricAnalysis(dmat,data): Calculate the metric from the design matrix; get
> various derived quantities from it & the data via Cholesky.";
>
> static PyObject *
> vba_metricAnalysis (PyObject *self, PyObject *args) {
> int mdims[2], pdims[1];
> int nmod, nsamp, a, b, i, m;
> PyObject *input1, *input2;
> PyArrayObject *dmat, *smpls;
> PyArrayObject *metric, *cfmetric, *proj, *amps;
> double *dmdata, *mdata, *cfdata, *diag, *pdata, *sdata, *adata;
> double sum, rdet;
>
> /** Parse the design matrix & data; make them contiguous. */
> Py_TRY(PyArg_ParseTuple(args, "OO", &input1, &input2));
> dmat = (PyArrayObject *)
> PyArray_ContiguousFromObject(input1, PyArray_DOUBLE, 2, 2);
> if (dmat == NULL) return NULL;
At this point you must DECREF dmat before leaving.
> smpls = (PyArrayObject *)
> PyArray_ContiguousFromObject(input2, PyArray_DOUBLE, 1, 1);
> if (smpls == NULL) return NULL;
### Memory leak here if you return because smpls is NULL because dmat has
not been DECREF'd. Change to:
if (smpls == NULL) {
PyDECREF(dmat);
return NULL;
}
>
> /** Check that their dimensions are consistent. */
> if (dmat->dimensions[1] != smpls->dimensions[0]) {
> PyErr_SetString(PyExc_ValueError, "inconsistent data dimensions!");
> return NULL;
> }
>
### Another Memory leak here since you need to DECREF both dmat and smpls
before leaving.
> /** Store the # of models & samples; make vector references to dmat & data. */
> nmod = dmat->dimensions[0];
> nsamp = dmat->dimensions[1];
> dmdata = DDATA(dmat);
> sdata = DDATA(smpls);
>
> /** Calculate the metric. */
> mdims[0] = nmod;
> mdims[1] = nmod;
> metric = (PyArrayObject *)PyArray_FromDims(2, mdims, PyArray_DOUBLE);
> if (metric == NULL) return NULL;
### Another Memory leak here, dmat and smpls need to be DECREF'd before
returnning. From this point on metric also needs to be DECREF'd.
> mdata = DDATA(metric);
> for (a=0; a<nmod; a++) {
> for (b=0; b<nmod; b++) {
> sum = 0.;
> for (i=0; i<nsamp; i++) {
> sum += *(dmdata+a*nsamp+i) * *(dmdata+b*nsamp+i);
> }
> *(mdata+a*nmod+b) = sum;
> }
> }
>
> /** Calculate the projections of the data on the models. */
> pdims[0] = nmod;
> proj = (PyArrayObject *)PyArray_FromDims(1, pdims, PyArray_DOUBLE);
--- Possible segfault here because no checking is done for NULL.
> pdata = DDATA(proj);
> for (a=0; a<nmod; a++) {
> sum = 0.;
> for (i=0; i<nsamp; i++) {
> sum += *(dmdata+a*nsamp+i) * *(sdata+i);
> }
> *(pdata+a) = sum;
> }
>
> /** Perform a Cholesky factorization of the metric. Calculate
> *** the sqrt(determinant) along the way. */
> cfmetric = (PyArrayObject *)PyArray_Copy(metric);
> if (cfmetric == NULL) return NULL;
### Another Memory leak possibility on error.
> cfdata = DDATA(cfmetric);
> diag = (double *) malloc(nmod*sizeof(double));
> rdet = 1.;
> for (a=0; a<nmod; a++) {
> for (b=a; b<nmod; b++) {
> sum = *(cfdata+a*nmod+b);
> for (m=a-1; m>=0; m--) {
> sum -= *(cfdata+a*nmod+m) * *(cfdata+b*nmod+m);
> }
> if (a == b) {
> if (sum <= 0.) {
> PyErr_SetString(PyExc_ValueError,
> "metric is not positive definite!");
> return NULL;
> }
> *(diag+a) = sqrt(sum);
> rdet = rdet * *(diag+a);
> } else *(cfdata+b*nmod+a) = sum/ *(diag+a);
> }
> }
>
> /** We have the factorization in the lower triangle and diag;
> *** replace the diagonal and zero the upper triangle. */
> for (a=0; a<nmod; a++) {
> for (b=a; b<nmod; b++) {
> if (a == b) {
> *(cfdata+a*nmod+a) = *(diag+a);
> } else *(cfdata+a*nmod+b) = 0.;
> }
> }
>
> /** Calculate the amplitude estimates by backsubstitution. */
> pdims[0] = nmod;
> amps = (PyArrayObject *)PyArray_FromDims(1, pdims, PyArray_DOUBLE);
--- Possible segfault here (no check for NULL result).
> adata = DDATA(amps);
> for (a=0; a<nmod; a++) {
> sum = *(pdata+a);
> for (b=a-1; b>=0; b--) {
> sum -= *(cfdata+a*nmod+b) * *(adata+b);
> }
> *(adata+a) = sum / *(cfdata+a*nmod+a);
> }
> for (a=nmod-1; a>=0; a--) {
> sum = *(adata+a);
> for (b=a+1; b<nmod; b++) {
> sum -= *(cfdata+b*nmod+a) * *(adata+b);
> }
> *(adata+a) = sum / *(cfdata+a*nmod+a);
> }
>
> /** Calculate the sufficient statistic. */
> sum = 0.;
> for (a=0; a<nmod; a++) {
> sum += *(adata+a) * *(pdata+a);
> }
>
> /** Clean up and return everything in a tuple. */
> free(diag);
> Py_DECREF(dmat);
> Py_DECREF(smpls);
> return Py_BuildValue("OOfOOf",metric,cfmetric,rdet,proj,amps,sum);
### Big Memory leak here. Using the "O" indicator in Py_BuildValue
INCREF's the object before returning it. But, since these objects already
have an INCREMENTED reference count you are incrementing the reference
count by two for these object. They will never go away and release their
memory.
The character you want to use is "N" and it was introduced in Python
1.5.2 (don't forget the PyArray_Return()):
return Py_BuildValue("NNfNNf",PyArray_Return(metric),
PyArray_Return(cfmetric),rdet,
PyArray_Return(proj),
PyArray_Return(amps),sum);
Alternatively, you could use the following construct:
retval = Py_BuildValue("OOfOOf",...blah...)
Py_DECREF(metric);
Py_DECREF(cfmetric);
Py_DECREF(proj);
Py_DECREF(amps);
return retval;
I prever the "N" option. Either of the last two options will get rid of
your memory leaks for the cases when the subroutine works correctly. If
it fails on any of the array construction calls after dmat, then you will
still get memory leaks because of failure to DECREF the
previously-constructed arrays.
One general method for handling this error-failure case is to define a
"fail" block that Py_XDECREF's all the arrayobjects (which have been
pre-initialized to NULL), so that when something fail's you can say
If (XXXX == NULL) goto fail and get all the DECREF's done correctly.
I use this construct a lot in my code. It is described in the Python
documentation as an appropriate use of goto in C.
Good luck,
I've found that at first the reference counting was hard, but it becomes
second nature, fairly quickly.
Have a great day,
Travis Oliphant
More information about the Python-list
mailing list