v.example leaks memory in db_open_select_cursor

[karme]

example valgrind run:

==8090== 570,258 bytes in 569,794 blocks are definitely lost in loss record 29 of 31
==8090==    at 0x4023E8C: realloc (vg_replace_malloc.c:429)
==8090==    by 0x4060670: db_realloc (alloc.c:72)
==8090==    by 0x4064B74: db_enlarge_string (string.c:116)
==8090==    by 0x406713A: db__recv_string (xdrstring.c:93)
==8090==    by 0x4066294: db__recv_column_definition (xdrcolumn.c:29)
==8090==    by 0x40675D8: db__recv_table_definition (xdrtable.c:35)
==8090==    by 0x40FC4FB: db_open_select_cursor (c_openselect.c:55)
==8090==    by 0x80499AC: main (main.c:165)

The problem is that each call to db_open_select_cursor => each database query leaks memory.

It seems it doesn't matter which database backend you use.

[karme]

proposed patch

[karme]

"proposed patch" is not quite true. I am not really sure - please review. Thanks.

[glynn]

Replying to karme:

"proposed patch" is not quite true. I am not really sure - please review. Thanks.

The patch looks reasonable enough, although it's essentially impossible to tell whether it will break anything. db_get_string() returns the pointer, and if anything holds onto the pointer after db_free_column() returns, it will break.

My inclination is to apply the patch. Unlike most GRASS memory "leaks", this one could be significant. I.e. if it leaks memory for each row fetched, it would impose a limit on the amount of data retrieved from a query.

[karme]

Replying to glynn:

My inclination is to apply the patch. Unlike most GRASS memory "leaks", this one could be significant. I.e. if it leaks memory for each row fetched, it would impose a limit on the amount of data retrieved from a query.

Well it does not leak memory for each row fetched, but for each query. But at the moment I am writing a v.out.osm that in the end does a query for each line => you run out of memory quickly. It would be really nice to get this fixed - maybe also in the stable branch? and maybe in debian/lenny?

A quick grep for db_open_select_cursor gives many hits and some of them are in a inner loop. For example: v.out.ogr (in mk_att), d.vect (attr.c), v.label, ...

[glynn]

Replying to karme:

A quick grep for db_open_select_cursor gives many hits and some of them are in a inner loop. For example: v.out.ogr (in mk_att), d.vect (attr.c), v.label, ...

These modules should probably be making a single SELECT statement and storing the results in a btree (or similar), rather than executing a separate query for each line.

[hamish]

Replying to karme:

maybe also in the stable branch?

if a fix makes it into trunk it is highly likely we will backport it to the stable branch.

and maybe in debian/lenny?

I doubt it. Lenny is now frozen and this is not a debian RC bug. We are pretty much locked in to 6.2.3 there, but keep a close eye out for a 6.3.0 (or 6.3.svn) version for Lenny from backports.org soon after lenny is released. A 6.3.svn package will contain critical backported fixes, but those are few. For the most part normal bug fixes will first see daylight with the upcoming 6.4.

Hamish

[karme]

I would suggest to apply this patch.

[neteler]

Fixed in GRASS 7 (r35066), 6.4.svn and 6.4.0svn (for RC2).

Markus