memory leak in function msOGRFileOpen

[linuxsch]

there have memory leak in function msOGRFileOpen.

==4650== 
==4650== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 5 from 2)
==4650== malloc/free: in use at exit: 1,089 bytes in 34 blocks.
==4650== malloc/free: 34,791 allocs, 34,757 frees, 173,579,917 bytes allocated.
==4650== For counts of detected errors, rerun with: -v
==4650== searching for pointers to 34 not-freed blocks.
==4650== checked 1,735,624 bytes.
==4650== 
==4650== 32 (16 direct, 16 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 14
==4650==    at 0x4904A06: malloc (vg_replace_malloc.c:149)
==4650==    by 0x4DBA312: CPLPushErrorHandler (in /usr/local/lib/libgdal.so.1.11.4)
==4650==    by 0x42EB3D: msOGRFileOpen(layer_obj*, char const*) (in /home/gsm/program/testmap_5x/src/testmap_5x)
==4650==    by 0x42F1D5: msOGRLayerOpen (in /home/gsm/program/testmap_5x/src/testmap_5x)
==4650==    by 0x40C3C9: myQueryByPoint(map_obj*, std::string, std::string, int, pointObj, double, std::list<MapQueryResult, std::allocator<MapQueryResult> >&) (myMapQuery.cpp:278)
==4650==    by 0x40CF99: QueryInArea(map_obj*, double, double, std::string, std::string, int) (testmap_5x.cpp:19)
==4650==    by 0x40D56A: main (testmap_5x.cpp:109)

[dmorissette]

This specific one is a one-time leak of 32 bytes to set a OGR Error handler. I'll check if we could unset this in msCleanup() but I don't think this one is serious and I wouldn't worry about it too much.

However, I see that there were a total of "1,089 bytes in 34 blocks" in use at exit... so what about the other 33 blocks of leaked memory? Were they in the MapServer code or in your own program?

[aboudreault]

I'm going to report the CPLPushErrorHandler() leak to the developers since it's not a part of mapserver. However, during the tests I did for this ticket, I've notice a few other leaks that I'm going to investigate in:

==25370== 59 (8 direct, 51 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 8                                                                                                  
==25370==    at 0x4026FDE: malloc (vg_replace_malloc.c:207)                                                                                                                                       
==25370==    by 0x40F1B0B: msOGRGetValues(layer_obj*, void*) (mapogr.cpp:568)                                                                                                                     
==25370==    by 0x40F3E7C: msOGRFileGetShape(layer_obj*, shapeObj*, long, ms_ogr_file_info_t*, int) (mapogr.cpp:1924)                                                                             
==25370==    by 0x40F3F7C: msOGRLayerResultGetShape(layer_obj*, shapeObj*, int, long) (mapogr.cpp:2621)                                                                                           
==25370==    by 0x40FA801: msLayerResultsGetShape (maplayer.c:160)                                                                                                                                
==25370==    by 0x409E4F1: msReturnNestedTemplateQuery (maptemplate.c:3733)                                                                                                                       
==25370==    by 0x4091ABC: msReturnTemplateQuery (maptemplate.c:255)
==25370==    by 0x80528EE: main (mapserv.c:1817)
==25370==
==25370==
==25370== 215,648 (384 direct, 215,264 indirect) bytes in 1 blocks are definitely lost in loss record 7 of 8
==25370==    at 0x40270FC: realloc (vg_replace_malloc.c:429)
==25370==    by 0x40E0E92: msAddLineDirectly (mapprimitive.c:325)
==25370==    by 0x40F36C3: ogrGeomLine(void*, shapeObj*, int) (mapogr.cpp:361)
==25370==    by 0x40F342B: ogrGeomLine(void*, shapeObj*, int) (mapogr.cpp:294)
==25370==    by 0x40F342B: ogrGeomLine(void*, shapeObj*, int) (mapogr.cpp:294)
==25370==    by 0x40F3BC8: ogrConvertGeometry(void*, shapeObj*, MS_LAYER_TYPE) (mapogr.cpp:425)
==25370==    by 0x40F3E11: msOGRFileGetShape(layer_obj*, shapeObj*, long, ms_ogr_file_info_t*, int) (mapogr.cpp:1903)
==25370==    by 0x40F3F7C: msOGRLayerResultGetShape(layer_obj*, shapeObj*, int, long) (mapogr.cpp:2621)
==25370==    by 0x40FA801: msLayerResultsGetShape (maplayer.c:160)
==25370==    by 0x409E4F1: msReturnNestedTemplateQuery (maptemplate.c:3733)
==25370==    by 0x4091ABC: msReturnTemplateQuery (maptemplate.c:255)
==25370==    by 0x80528EE: main (mapserv.c:1817)
==25370==
==25370==
==25370== 464,480 bytes in 10 blocks are possibly lost in loss record 8 of 8
==25370==    at 0x4026FDE: malloc (vg_replace_malloc.c:207)
==25370==    by 0x40F34DD: ogrGeomLine(void*, shapeObj*, int) (mapogr.cpp:322)
==25370==    by 0x40F342B: ogrGeomLine(void*, shapeObj*, int) (mapogr.cpp:294)
==25370==    by 0x40F342B: ogrGeomLine(void*, shapeObj*, int) (mapogr.cpp:294)
==25370==    by 0x40F3BC8: ogrConvertGeometry(void*, shapeObj*, MS_LAYER_TYPE) (mapogr.cpp:425)
==25370==    by 0x40F3E11: msOGRFileGetShape(layer_obj*, shapeObj*, long, ms_ogr_file_info_t*, int) (mapogr.cpp:1903)
==25370==    by 0x40F3F7C: msOGRLayerResultGetShape(layer_obj*, shapeObj*, int, long) (mapogr.cpp:2621)
==25370==    by 0x40FA801: msLayerResultsGetShape (maplayer.c:160)
==25370==    by 0x409E4F1: msReturnNestedTemplateQuery (maptemplate.c:3733)
==25370==    by 0x4091ABC: msReturnTemplateQuery (maptemplate.c:255)
==25370==    by 0x80528EE: main (mapserv.c:1817)
==25370==
==25370== LEAK SUMMARY:
==25370==    definitely lost: 400 bytes in 3 blocks.
==25370==    indirectly lost: 215,323 bytes in 41 blocks.
==25370==      possibly lost: 464,480 bytes in 10 blocks.
==25370==    still reachable: 28 bytes in 7 blocks.
==25370==         suppressed: 0 bytes in 0 blocks.

[aboudreault]

The last memory leak was related to templates. When the template file is not found, the shapes wasn't freed.

Fixed and committed in r9722.

[warmerdam]

For reference, I don't feel the error handler leak is worth pursuing.

[aboudreault]

warmerdam, I don't know really how CPL works... but I've seen that we use CPLPushErrorHandler function in mapogr.cpp and mapgdal.cpp. Would it have a specific place in the code that we should call CPLPopErrorHandler ?

[warmerdam]

Alan,

On review, I think msOGRCleanup() ought to call CPLPopErrorHandler() and that might cleanup the allocation in an appropriate way. Do you mind trying that?

[aboudreault]

FrankW, I tried to add a call to CPLPopErrorHandler() in msOGRCleanup(), but this hasn't change anything. Closing this ticket since we don't feel the error handler leak is worth pursuing.