tileindex : possiblity of memeory leak

[assefa]

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 ?

[sdlime]

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

[zjames]

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

[sdlime]

Replying to 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

[zjames]

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.

[assefa]

:) 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)

[assefa]

patch commited in r6247.

[sdlime]

Assuming Assefa's patch resolved this one. Closing.

Steve