tileindex : possiblity of memeory leak

[tbonfort]

Reporter: assefa Date: 2007/06/25 - 22:08 In function msTiledSHPNextShape in mapshape.c, almost at the end of the function, there is a test on the filter of the layer (aound 1847) : if((filter_passed = msEvalExpression(&(layer->filter), layer->filteritemindex, values, layer->numitems)) != MS_TRUE) ...

after that call, the function always reads the shape using msSHPReadShape(tSHP->shpfile->hSHP, i, shape) but this shape seems to never be freed since the function loops again if the filter_passed = 0.

Is it necessary to read the shape if the filter_passed=0 ? if yes we should add something like this before the end of the loop: if (!filter_passed) msFreeShape(shape);

Are my assumptions correct ?

[tbonfort]

Author: sdlime Date: 2007/06/28 - 16:13 Man, you're taking me to code written 4 years ago. I assume you weren't just reading through this stuff for grins. Why do you suspect the leak here? Did you try your fix?

Steve

[tbonfort]

Author: zjames Date: 2007/06/28 - 16:38 We found this leak using valgrind after a mapscript process consumed 4GB on our client's machine.

[tbonfort]

Author: sdlime Date: 2007/06/28 - 16:44 Replying to [comment:3 zjames]:

We found this leak using valgrind after a mapscript process consumed 4GB on our client's machine.

That's a pretty serious leak then. Did you try the patch Assefa mentioned? I'll take a look ASAP but it will take a few hours to get my head around that code and if you've already tried and succeeded... ;-)

Steve

[tbonfort]

Author: zjames Date: 2007/06/28 - 16:49 Yes, Assefa's patch resolved the problem in that case. We were concerned there could be side effects. The leak was ~200k per draw with our tiled shapefiles - we were drawing many, many image tiles in the same php process. Otherwise, it's unlikely we would have noticed.

[tbonfort]

Author: assefa Date: 2007/06/28 - 16:50 :) I did not look for it. It came to us. Zak was using Mapserver to generate tiles and realized there was a memeory leak and run it through validring.

Here is what is happening : if you have a tileindex layer with a filter set, if a shpe does not satisify the filter criteria, the filter_passed variable is set to 0. But the function still reads the shape; the function loops back until it gets a shape that satisfy the filter. Memeory is lost when doing msSHPReadShape since shapes that do not meet filter criteria are never freed. That is my understanding.

I have a test map files with data to reproduce this. I can send it if you like.

I just saw the prevous comment when writing this. I can certainly commit the patch that we have here.

Here is the result from valigrind :

==17860== 196,768 (98,384 direct, 98,384 indirect) bytes in 6,149 blocks are definitely lost in loss record 16 of 16 ==17860== at 0x4904B7E: malloc (vg_replace_malloc.c:149) ==17860== by 0x453632: msSHPReadShape (mapshape.c:1212) ==17860== by 0x454CE9: msTiledSHPNextShape (mapshape.c:1853) ==17860== by 0x425862: msDrawVectorLayer (mapdraw.c:880) ==17860== by 0x425AAA: msDrawLayer (mapdraw.c:683) ==17860== by 0x42681D: msDrawMap (mapdraw.c:419) ==17860== by 0x408A6B: main (shp2img.c:200)

[tbonfort]

Author: assefa Date: 2007/06/29 - 14:22 patch commited in 263fd2cadf039056dc6f590ce4493c57d811bb1d (r6247).

[tbonfort]

Author: sdlime Date: 2007/08/08 - 08:10 Assuming Assefa's patch resolved this one. Closing.

Steve