Opened 11 years ago

Closed 11 years ago

#273 closed defect (fixed)

v.example leaks memory in db_open_select_cursor

Reported by: karme Owned by: grass-dev@…
Priority: major Milestone: 6.4.0
Component: Database Version: svn-trunk
Keywords: Cc:
CPU: x86-32 Platform: Linux

Description

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.

Attachments (1)

grass-memleak.patch (412 bytes) - added by karme 11 years ago.
proposed patch

Download all attachments as: .zip

Change History (9)

comment:1 Changed 11 years ago by karme

Component: defaultDatabase

Changed 11 years ago by karme

Attachment: grass-memleak.patch added

proposed patch

comment:2 Changed 11 years ago by karme

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

comment:3 in reply to:  2 ; Changed 11 years ago by 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.

comment:4 in reply to:  3 ; Changed 11 years ago by 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, ...

comment:5 in reply to:  4 Changed 11 years ago by 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.

comment:6 in reply to:  4 Changed 11 years ago by 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

comment:7 Changed 11 years ago by karme

I would suggest to apply this patch.

comment:8 Changed 11 years ago by neteler

Resolution: fixed
Status: newclosed

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

Markus

Note: See TracTickets for help on using tickets.