Opened 9 years ago

Closed 15 months ago

#2055 closed defect (fixed)

[PATCH - libtiff] Leak in GDALDefaultOverviews::HaveMaskFile ?

Reported by: Even Rouault Owned by: warmerdam
Priority: normal Milestone: 2.1.0
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords:


When running Valgrind on (mask_2 and mask_3 tests can be disabled), I get the following leak reported :

==4297== 272 bytes in 2 blocks are definitely lost in loss record 114 of 136
==4297==    at 0x4022765: malloc (vg_replace_malloc.c:149)
==4297==    by 0x4701CCC: VSIMalloc (cpl_vsisimple.cpp:280)
==4297==    by 0x469D504: _TIFFmalloc (tif_vsi.c:156)
==4297==    by 0x469FF42: TIFFInitZIP (tif_zip.c:406)
==4297==    by 0x465409D: TIFFSetCompressionScheme (tif_compress.c:172)
==4297==    by 0x4654C61: _TIFFVSetField (tif_dir.c:196)
==4297==    by 0x4655E9A: TIFFVSetField (tif_dir.c:696)
==4297==    by 0x4655E53: TIFFSetField (tif_dir.c:682)
==4297==    by 0x465EEBE: TIFFReadDirectory (tif_dirread.c:3412)
==4297==    by 0x468D72C: TIFFClientOpen (tif_open.c:464)
==4297==    by 0x46BB9E1: XTIFFClientOpen (xtiff.c:174)
==4297==    by 0x46A02EB: VSI_TIFFOpen(char const*, char const*) (tifvsi.cpp:172)
==4297==    by 0x455BA73: GTiffDataset::Open(GDALOpenInfo*) (geotiff.cpp:2810)
==4297==    by 0x46CF5C9: GDALOpen (gdaldataset.cpp:1774)
==4297==    by 0x46D14CA: GDALDefaultOverviews::HaveMaskFile(char**, char const*) (gdaldefaultoverviews.cpp:760)
==4297==    by 0x46D2789: GDALDefaultOverviews::BuildOverviews(char const*, char const*, int, int*, int, int*, int (*)(double, char const*, void*), void*) (gdaldefaultoverviews.cpp:488)
==4297==    by 0x46D01BE: GDALDataset::IBuildOverviews(char const*, int, int*, int, int*, int (*)(double, char const*, void*), void*) (gdaldataset.cpp:1197)
==4297==    by 0x46D007F: GDALDataset::BuildOverviews(char const*, int, int*, int, int*, int (*)(double, char const*, void*), void*) (gdaldataset.cpp:1146)
==4297==    by 0x46D0133: GDALBuildOverviews (gdaldataset.cpp:1174)
==4297==    by 0x442FE5C: _wrap_Dataset_BuildOverviews (gdal_wrap.cpp:3255)

The leak is caused by something in mask_5. (GDAL built with internal libtiff)

Change History (4)

comment:1 Changed 9 years ago by Even Rouault

Summary: Leak in GDALDefaultOverviews::HaveMaskFile ?[PATCH - libtiff] Leak in GDALDefaultOverviews::HaveMaskFile ?

I've identified the leak as being the compression state structure not freed by TIFFFreeDirectory( hTIFF ) at line #102 of frmts/gtiff/tif_overview.c. The call to TIFFSetField( hTIFF, TIFFTAG_COMPRESSION, nCompressFlag ) at line #117 allocates again the compression state structure.

The fix I propose is the following patch to libtiff :

Index: frmts/gtiff/libtiff/tif_dir.c
--- frmts/gtiff/libtiff/tif_dir.c       (révision 13728)
+++ frmts/gtiff/libtiff/tif_dir.c       (copie de travail)
@@ -1027,6 +1027,7 @@
        TIFFDirectory *td = &tif->tif_dir;
        int            i;

+        (*tif->tif_cleanup)(tif);
        _TIFFmemset(td->td_fieldsset, 0, FIELD_SETLONGS);

By looking at libtiff internal calls to TIFFFreeDirectory, I've observed that for almost all occurences a call to (*tif->tif_cleanup)(tif) is done a few lines before or after calling TIFFreeDirectory. So I imagine it would be maybe justified to remove all those calls and do the cleanup of the compression state in TIFFFreeDirectory.

comment:2 Changed 2 years ago by Jukka Rahkonen

Even, is there a reason to keep this ticket open?

comment:3 Changed 2 years ago by Even Rouault

I think the issue is still present. It is really a libtiff bug, but I'm a bit hesitant to apply the fix without studying all possible impacts.

comment:4 Changed 15 months ago by Even Rouault

Milestone: 2.1.0
Resolution: fixed
Status: newclosed

Instead of touching libtiff, I found the following fix/hack

trunk r32732 "GTiff: GTIFFWriteDirectory(): avoid memory leak of codec related memory (#2055)"

Note: See TracTickets for help on using tickets.