#70 memory leak in libmng_display.c


There's a memory leak in mng_display_image() that
shows up for grayscale images. All of the Delta-
grayN tests in the MNG test suite demonstrate the
problem - try, for example,

The problem is at libmng_display.c line 1526 (in

MNG_ALLOC (pData, pData->pRGBArow, pData->iRowsize);

This line is sometimes invoked with a valid, non-null
value (from a previous allocation) already in pData-

pRGBArow. Since this line just overwrites any
previous value, the old allocation is simply
forgotten, so it never gets freed, thus the leak.

Adding this just before the MNG_ALLOC seems to fix it:

if (pData->pRGBArow != 0)
MNG_FREE(pData, pData->pRGBArow, pData->iRowsize);

I'm not sure this is consistent with the overall
memory allocation strategy - hopefully someone
familiar with the code will look at it to make sure
there's not a deeper bug here. But this fix at least
plugs the memory leak for the cases I've found so far.


  • Nobody/Anonymous

    Logged In: NO

    Actually, the solution above does seem to cause some
    problematic side effects, as I feared.

    1. There seem to be about half a dozen places where
      pGBArow is freed and not nulled out, so if you run
      through any of those and then hit my fix above, you'll get
      a double free. Explicitly nulling out pRGBArow every time
      it's freed will take care of those.

    2. There's also one tricky situation where pRGBArow is
      saved and temporarily replaced with another buffer, then
      restored; this again leads to a double free, because the
      nulling out is undone by the restore. The fix for this is
      explicitly nulling out pRGBArow after it's saved, so that
      the saved copy is safely removed from consideration for
      freeing. This is in load_bkgdlayer() in libmng_display.c;
      the nulling out needs to go right after the huge block of
      saves at the top of the function. I wouldn't be surprised
      if this fix creates further complications in other code
      paths where it's assumed there's a non-null pRGBArow, but
      I've run through the MNG suite and at least those all seem
      to work.

  • Glenn Randers-Pehrson

    Logged In: YES
    Originator: NO

    Adding this just before the MNG_ALLOC seems to fix it:

    if (pData->pRGBArow != 0)
    MNG_FREE(pData, pData->pRGBArow, pData->iRowsize);

    Maybe we could put the test inside MNG_ALLOC(), and throw a warning
    if MNG_ALLOC() decides it needs to MNG_FREE() the target. We'd also
    need to fix MNG_FREE() so it always nulls out the target.

    This will take some work to get it right, but once done, I think
    the memory management will be a lot more robust.



Cancel  Add attachments

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

No, thanks