Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#2729 closed defect (fixed)

msLoadExpressionString() cause memory leaks

Reported by: aboudreault Owned by: sdlime
Priority: normal Milestone: 5.4 release
Component: MapServer C Library Version: svn-trunk (development)
Severity: normal Keywords:
Cc: dmorissette

Description

Every call of msLoadExpressionString() function cause a memory leak. This function is used is few parts in MapServer. I noticed this bug during a test i made. The map that i used for my test had only one layer of type annotation, connectiontype ogr (mapinfo) and styleitem auto. This layer contains around 40 labels. In the function msOGRLayerGetAutoStyle() in mapogr.cpp, the call of the msLoadExpressionString() is made for each label. This cause a lot a memory leaks. msLoadExpressionString() is also called in php/mapscript, in the function classObj->setText(). So, you can easily reproduce the bug with a simple layer, that is a label and try to change the text.

Example:

<?php
dl("php_mapscript.so");
$oMap = ms_newMapObj("/opt/fgs/www/test/myMap.map");
echo $oMap->numlayers;
$oLayer = $oMap->GetLayer(0);
$oClass = $oLayer->getClass(0);
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
$oClass->setText("TEXT HASASASASA");
echo "done";
?>

You'll see memory leaks with php-cli and valgrind. Here is my valgrind's output:

==9956== ERROR SUMMARY: 27 errors from 7 contexts (suppressed: 131 from 1)                                             
==9956== malloc/free: in use at exit: 17,348 bytes in 24 blocks.                                                       
==9956== malloc/free: 15,288 allocs, 15,264 frees, 1,866,624 bytes allocated.                                          
==9956== For counts of detected errors, rerun with: -v                                                                 
==9956== searching for pointers to 24 not-freed blocks.                                                                
==9956== checked 748,792 bytes.
==9956==
==9956==
==9956== 17,084 (528 direct, 16,556 indirect) bytes in 11 blocks are definitely lost in loss record 3 of 4
==9956==    at 0x4022AB8: malloc (vg_replace_malloc.c:207)
==9956==    by 0x4B7BFEC: ???
==9956==    by 0x4B7C478: ???
==9956==    by 0x4B7C54B: ???
==9956==    by 0x4B9344E: ???
==9956==    by 0x4B46BEE: ???
==9956==    by 0x4B4075D: ???
==9956==    by 0x82C211B: zend_do_fcall_common_helper_SPEC (zend_vm_execute.h:200)
==9956==    by 0x82B4888: execute (zend_vm_execute.h:92)
==9956==    by 0x82977E9: zend_execute_scripts (zend.c:1134)
==9956==    by 0x825720C: php_execute_script (main.c:2004)
==9956==    by 0x831020D: main (php_cli.c:1140)
==9956==
==9956== LEAK SUMMARY:
==9956==    definitely lost: 528 bytes in 11 blocks.
==9956==    indirectly lost: 16,556 bytes in 11 blocks.
==9956==      possibly lost: 0 bytes in 0 blocks.
==9956==    still reachable: 264 bytes in 2 blocks.
==9956==         suppressed: 0 bytes in 0 blocks.

Change History (8)

comment:1 by aboudreault, 16 years ago

The valgrind's output is not very useful, here is a better output generated with valgrind and mapserver with the OGR MapInfo Layer:

==13449== ERROR SUMMARY: 3 errors from 1 contexts (suppressed: 107 from 1)                                                                           
==13449== malloc/free: in use at exit: 19,347 bytes in 84 blocks.                                                                                    
==13449== malloc/free: 15,017 allocs, 14,936 frees, 2,102,099 bytes allocated.                                                                       
==13449== For counts of detected errors, rerun with: -v                                                                                              
==13449== searching for pointers to 84 not-freed blocks.                                                                                             
==13449== checked 1,702,924 bytes.                                                                                                                   
==13449==                                                                                                                                            
==13449==                                                                                                                                            
==13449== 16 (8 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 4                                                      
==13449==    at 0x4022AB8: malloc (vg_replace_malloc.c:207)                                                                                          
==13449==    by 0x458E90C: VSIMalloc (cpl_vsisimple.cpp:300)                                                                                         
==13449==    by 0x457BB09: CPLPushErrorHandler (cpl_error.cpp:678)                                                                                   
==13449==    by 0x4083334: msOGRFileOpen(layer_obj*, char const*) (mapogr.cpp:806)
==13449==    by 0x408547A: msOGRLayerOpen (mapogr.cpp:1563)
==13449==    by 0x4085534: msOGRLayerOpenVT(layer_obj*) (mapogr.cpp:1660)
==13449==    by 0x408BE61: msLayerOpen (maplayer.c:88)
==13449==    by 0x40A5D9E: msDrawVectorLayer (mapdraw.c:829)
==13449==    by 0x40A6432: msDrawLayer (mapdraw.c:738)
==13449==    by 0x40A7428: msDrawMap (mapdraw.c:441)
==13449==    by 0x804E430: main (mapserv.c:1434)
==13449==
==13449==
==13449== 19,331 (1,968 direct, 17,363 indirect) bytes in 41 blocks are definitely lost in loss record 3 of 4
==13449==    at 0x4022AB8: malloc (vg_replace_malloc.c:207)
==13449==    by 0x4055FEC: msyyalloc (maplexer.c:4320)
==13449==    by 0x4056478: msyy_create_buffer (maplexer.c:3859)
==13449==    by 0x405654B: msyyrestart (maplexer.c:3798)
==13449==    by 0x406D44E: msLoadMap (mapfile.c:4654)
==13449==    by 0x804B6BA: loadMap (mapserv.c:203)
==13449==    by 0x804D7DD: main (mapserv.c:1214)
==13449==
==13449== LEAK SUMMARY:
==13449==    definitely lost: 1,976 bytes in 42 blocks.
==13449==    indirectly lost: 17,371 bytes in 42 blocks.
==13449==      possibly lost: 0 bytes in 0 blocks.
==13449==    still reachable: 0 bytes in 0 blocks.
==13449==         suppressed: 0 bytes in 0 blocks.

Steve, i tryed to check why msLoadExpressionString() could memory leak, but i don't know so much the maplexer.c/maplexer.l code.

comment:2 by dmorissette, 16 years ago

Steve, we'll need your help with this one.

The Valgrind output quoted here is not very convincing, but our tests show that there would be a leak of the data structures allocated by msyy_create_buffer() when msLoadExpressionString() is called either by MapScript code, or by mapogr.cpp's STYLEITEM AUTO handling to assign a constant string to an expressionObj. We tried to trace what is happening in maplexer but do not understand it well enough to figure the source of the problem.

We've found that every call to msLoadExpressionString() results in one leak... a simple way to reproduce this is to write a MapScript script that makes multiple calls to $class->setText("somestring")... each call will result in a leak.

Any idea?

comment:3 by sdlime, 16 years ago

Working on it. Here's the output from 'leaks'. Seems like freeExpression isn't working quite right although I noticed that the last setText string ('testy10') isn't leaked so the final cleanup seems ok. Hmmmm...

Steve

Process 1051: 27174 nodes malloced for 1706 KB Process 1051: 18 leaks for 608 total leaked bytes. Leak: 0x301d40 size=48

0xa0564de0 0x002ec000 0x002ec180 0x00004000 .MV..........@.. 0x00000181 0x00000001 0x00000000 0x00000001 ................ 0x00000001 0x00000000 0x00000001 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_new_mapObj | msLoadMap | msyyrestart | msyy_create_buffer | malloc | malloc_zone_malloc

Leak: 0x452900 size=48

0x00000000 0x0038f460 0x0038f466 0x00000006 ....`.8.f.8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x452b90 size=48

0x00000000 0x0038f7d0 0x0038f7d6 0x00000006 ......8...8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x452e30 size=48

0x00000000 0x0038f7c0 0x0038f7c6 0x00000006 ......8...8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x4530d0 size=48

0x00000000 0x0038f780 0x0038f786 0x00000006 ......8...8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x453360 size=48

0x00000000 0x0038f770 0x0038f776 0x00000006 ....p.8.v.8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x453600 size=48

0x00000000 0x00390010 0x00390016 0x00000006 ......9...9..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x4538c0 size=48

0x00000000 0x00390000 0x00390006 0x00000006 ......9...9..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x453b80 size=48

0x00000000 0x0038ffc0 0x0038ffc6 0x00000006 ......8...8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x00000000 0x00000000 0x00000000 0x00000001 ................ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x4548b0 size=48

0x00000000 0x0038ffb0 0x0038ffb6 0x00000006 ......8...8..... 0x00000006 0x00000001 0x00000000 0x00000001 ................ 0x29303237 0x00000000 0x00000000 0x00000001 720)............ Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | msyy_scan_buffer | malloc | malloc_zone_malloc

Leak: 0x3012f0 size=16 string 'main'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x1fdd | perl_parse | S_parse_body | Perl_gv_fetchpv | Perl_savepvn | Perl_safesysmalloc | malloc | malloc_zone_malloc

Leak: 0x38f460 size=16 string 'testy9'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

Leak: 0x38f770 size=16 string 'testy5'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

Leak: 0x38f780 size=16 string 'testy6'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

Leak: 0x38f7c0 size=16 string 'testy7'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

Leak: 0x38f7d0 size=16 string 'testy8'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

Leak: 0x38ffb0 size=16 string 'testy1'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

Leak: 0x38ffc0 size=16 string 'testy2'

Call stack: [thread 0xa0562fa0]: | 0x1ee6 | 0x204e | perl_run | Perl_runops_standard | Perl_pp_entersub | _wrap_classObj_setText | loadExpressionString | msyylex | msyy_scan_string | msyy_scan_bytes | malloc | malloc_zone_malloc

comment:4 by sdlime, 16 years ago

Fixed in the trunk (r2864). The issue is that the lexer wasn't being cleaned up properly between string tokenizing. Affected expression setting from mapscript and string-based mapfile parsing from a URL or MapScript. Expression evaluation within already had this done correctly which is why this didn't surface until now. Might want to consider a backport to 5.2 on this one. If you guys could verify the fix I'll do that...

Steve

comment:5 by dmorissette, 16 years ago

Thanks Steve! Alan will test the fix when he's back in the office tomorrow... and then I agree with a backport to 5.2 since that leak can affect uses of OGR STYLEITEM AUTO quite a bit in long-running processes.

(For the record, the correct SVN trunk revision r7864)

comment:6 by aboudreault, 16 years ago

I confirm that the patch solve the problem. Thanks!

comment:7 by sdlime, 16 years ago

Resolution: fixed
Status: newclosed

Fixed in 5.2 branch as well (r7868). Dan, can you commit a new maplexer.c?

Steve

comment:8 by dmorissette, 16 years ago

New maplexer.c committed in branch-5-2 r7872.

Note: See TracTickets for help on using tickets.